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

Fix unintentionally scrolling sidebar #12456

Closed
wants to merge 1 commit into from

Conversation

timrspratt
Copy link
Contributor

@timrspratt timrspratt commented Apr 23, 2024

When using certain field components that have <template> tags in for modals and alpine collapse functionality, the sidebar can end up scrolling incorrectly over the sticky header.

The header is sticky with a z-20, so this PR adds the same to the sidebar.

Top header wrapper:
Screenshot 2024-04-23 at 12 25 58

Sidebar wrapper before:
image

Sidebar wrapper after:
image

Video showing issue:

Screen.Recording.2024-04-23.at.12.24.21.mov

I have checked scenarios for mobile menu, top navigation, collapse navigation and all appear to be correct.

@danharrin danharrin added the ui label Apr 23, 2024
@danharrin danharrin added this to the v3 milestone Apr 23, 2024
Copy link
Member

@zepfietje zepfietje left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @timrspratt!

I think position fixed should be used instead, since the sticky position in this case would never act as relative since it's always at the top 0 already, right?

Also, could you please check if the collapsible sidebar animation on desktop still works as expected? Please post a screen recording of that here.

@zepfietje zepfietje changed the title make sidebar sticky Fix unintentionally scrolling sidebar Apr 24, 2024
@timrspratt
Copy link
Contributor Author

I'll have a play with fixed properly when I can. A quick try just now and it looks like w-full would also be needed.

@zepfietje
Copy link
Member

Are you able to pick this up again, @timrspratt?

@zepfietje
Copy link
Member

I'm going to close this PR, since there hasn't been any activity and requires some more testing before it can be merged.

Feel free to open a new PR when you've had a chance to look at my change request, @timrspratt. 🙂

@zepfietje zepfietje closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants