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

Sidebar Container: Run the fixed sidebar check after expanding the table of contents #410

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jun 22, 2023

Fixes #409 — When the page was loaded initially, the isSidebarWithinViewport check was run on the collapsed sidebar, so the height check in isSidebarWithinViewport passed and the sidebar is set as fixed. Then, a few lines later, the collapse/expand toggle logic expanded the sidebar on large screens, and now it could be too tall for the fixed sidebar behavior.

This fix just swaps the initial isSidebarWithinViewport to after the toggle button logic, so that the menu is in it's correct state when the height is measured.

To test:

@ryelle ryelle self-assigned this Jun 22, 2023
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Nice work, LGTM :shipit:

@ryelle ryelle merged commit b6b919e into trunk Jun 23, 2023
@ryelle ryelle deleted the fix/409-fixed-sidebar branch June 23, 2023 15:49
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.

Fixed sidebar creates loss of visible focus & unreachable items.
2 participants