-
Notifications
You must be signed in to change notification settings - Fork 18
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
Sidebar intelligent responsive show/hide it when crosses the breakpoint #536
Conversation
@wojtekn , @matt-west , I think you will love this PR 😉 . |
This is awesome @sejas ! 🔥 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well on Mac and Windows.
I noticed one small thing I'm not sure about. When I make the Studio window wide, then close the sidebar, and make it narrow and wide again, the sidebar reappears.
Should we save a session flag when the user closes the sidebar deliberately so we won't show it again when they resize the window back and forth?
I think it's more clear if the resize toggles the sidebar every time it crosses the breakpoint. If we disable it, it would be like having "button/feature" that sometimes doesn't work, for some reason. I'll merge this PR, but if you decide that the button should "disable/override" the resizing action, then I'll be happy to create another PR. |
Makes sense @sejas , thanks for sharing your perspective. |
Nice work @sejas, this is super cool!
I agree with @wojtekn here. We should remember if the user previously closed the sidebar and not re-open it automatically when they resize the window. This is consistent with the behaviour of Apple apps like Notes. Screen.Recording.2024-09-16.at.14.33.08.mov |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Related to:
Proposed Changes
Testing Instructions
Regular Case:
npm start
Second case:
Check this video, and let me know what you think.
4qWIWj.mp4
Pre-merge Checklist