Skip to content
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

Save sidebar state per session #1606

Merged
merged 3 commits into from
Sep 16, 2022
Merged

Save sidebar state per session #1606

merged 3 commits into from
Sep 16, 2022

Conversation

cw789
Copy link
Contributor

@cw789 cw789 commented Sep 16, 2022

Means sidebar stays open or close if you navigate.
For example using previous / next page navigation.

Close #1554

Additionally I needed to fix a CSS animation.
By doing so I moved the sidebar button 7px more from left & top into the page.
Found it unwanted to have a different height position between closed / opened sidebar.

Means sidebar stays open or close if you navigate.
For example using previous / next page navigation.
@josevalim
Copy link
Member

Can you please revert the button changes? I am pretty sure it was intentional in the design. :)

@cw789
Copy link
Contributor Author

cw789 commented Sep 16, 2022

If I keep the transition on body.sidebar-closed .sidebar-button the button will always initially fly from left to right if sidebar will be closed. If I just set the transition to nonethe button will jump 8px to the top on closing the sidebar.

But yes I see the intention now. It's so to have the button centered within the kept free space (for the button) on narrow screens. The 8px move up is just to have it visually aligned to the corner when closed or to the search input then opened.

@cw789
Copy link
Contributor Author

cw789 commented Sep 16, 2022

I've tested some things but do not find a real solution.
But I found out the left to right flying sidebar-button on initial page load is nothing new.
It already occurred on narrow screens with the current implementation.

So from my side it's ok to leave this behavior as is.

@josevalim josevalim merged commit dd4f391 into elixir-lang:main Sep 16, 2022
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@cw789 cw789 deleted the persist_sidebar_state branch September 16, 2022 16:04
@josevalim
Copy link
Member

Unfortunately this introduces a bug:

  1. If I close the sidebar
  2. Then I open it
  3. And then make the screen small, the sidebar no longer collapses automatically

I think we should only store the closed state. Can you please send a pull request? I will revert this PR meanwhile.

josevalim added a commit that referenced this pull request Oct 19, 2022
@cw789
Copy link
Contributor Author

cw789 commented Oct 19, 2022

I thought this behavior is what to expect.
No magic happening on rotation or size changes.

But ok on my side if this little magic is expected. I'll have a look.

@josevalim
Copy link
Member

Thank you!

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

Successfully merging this pull request may close these issues.

Feature to store sidebare state across reloads
2 participants