-
Notifications
You must be signed in to change notification settings - Fork 334
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
fix(sidebar): track user pref for sidebar state #1394
fix(sidebar): track user pref for sidebar state #1394
Conversation
state.sidebarPreference = userPref.CLOSED | ||
break | ||
case userPref.CLOSED: | ||
state.sidebarPreference = userPref.OPEN |
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.
Is this correct? If the user preference is closed, we are setting it to open? 🤔
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.
The control logic is correct. The function is called when the click
event tied to the toggle button fires. So whenever the user changes the state of the sidebar with the toggle button, we'll update their preference.
So, if you're preference was closed we'll set it open
And if you're preference was open, we'll set it to closed
And if you had no_pref, we'll set the pref to the state of the sidebar.
I've attached a short video showing the state changes while working with the sidebar
CB576ED3-41C0-4588-AC86-38EE7E9A2702.MP4
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.
would it be better if I called it togglePreference()
instead of setPreference()
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.
Beautiful video, thank you! 😍
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.
would it be better if I called it togglePreference() instead of setPreference()
Ah, good idea! I just merged the PR though, but feel free to submit an update. :)
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.
will do
💚 💙 💜 💛 ❤️ |
Thanks @josevalim I really appreciate your hard work. :) |
Issue: #1339
Resolves the issue with the state of the sidebar forcefully changing when the screen size updates.
Now, the sidebar will track the preference of the user.
When the preference is closed --> the sidebar will remain closed on window resize
When the preference is open (or no_pref) --> the sidebar will toggle according to screen width on resize