Skip to content

fix: ensure side panel closes on outside click on mobile device #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rahulptl165
Copy link

@rahulptl165 rahulptl165 commented Mar 7, 2025

Resolves #50 .

Checklist

Please ensure the following tasks are completed before submitting this pull request.

  • Read and understand the Code of Conduct.
  • Read and understood the licensing terms.
  • Searched for existing issues and pull requests before submitting this pull request.
  • Filed an issue (or an issue already existed) prior to submitting this pull request.
  • Rebased onto latest master.
  • Submitted against master branch.

Description

This PR enhances the SideMenu component to enure side panel closes on outside click on mobile device.

This pull request:

  • Updates the Drawer component to switch between persistent and temporary variants based on screen width (<= 640px is considered mobile).
  • Adds conditional onClose behavior: only triggers on mobile when the Drawer is temporary.
  • Introduces mobile-specific CSS to adjust the close button’s styling at max-width: 640px.

Related Issues

Does this pull request have any related issues? yes

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Video

Screen.Recording.2025-03-08.153138.1.mp4

No.


@stdlib-js/reviewers

@@ -798,12 +798,14 @@ class SideMenuDrawer extends React.Component {
* @returns {ReactElement} React element
*/
render() {
const isMobile = typeof window !== 'undefined' && window.innerWidth <= 640;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we already check whether the device is a mobile device. If so, we should be pushing that down, rather than creating this sort of custom check.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check the codebase to see if we can utilize the existing mobile device check, and update my pull request shortly.

@kgryte
Copy link
Member

kgryte commented Mar 8, 2025

To help with PR review, it would help if you can provide screenshots and/or GIFs demonstrating new UI behavior. Otherwise, it is hard to know whether the changes introduced in this PR are what is desired.

@rahulptl165
Copy link
Author

I have updated the pull request to include a video demonstrating the new UI behavior.

@rahulptl165
Copy link
Author

Hi @kgryte, I have update the code to use viewport-width module for mobile detection as it is implemented in app.jsx. its working same as shown in video.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: on small screens, clicking outside of the side panel should close the side panel
2 participants