-
Notifications
You must be signed in to change notification settings - Fork 17
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: fix sticky nav jumping on narrow screen #685
base: master
Are you sure you want to change the base?
Conversation
Not able to reproduce this as well :( |
|
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.
71px was used because of this issue, this issue shows up on some devices (Fabien has gift to reproduce it on everything he touches 😆), to repro open proposal page with 200% zoom in devtools and compare.
Screen.Recording.2024-09-02.at.19.00.09.mov
With this PR you can see content that's between Topnav and sticky menu on scroll.
This seems a very special edge case. I can not test this edge case, due to my screen size not reaching more than 800px wide on a 200% zoom. We could get rid of this issue by using 71px everywhere, and still have a 72px topnav, making everything overlapping by 1px, but this will just mask the issue. I will try another solution, by getting rid of all these placement using margin |
I see. In my case, can reproduce it only on a 75% zoom, all other zoom working well. Guess this is some scaling, since by changing the screen resolution, I also got the issue on a 125% zoom. |
Weirdly, we don't have this issue at all on the space settings page |
The quick fix we have now on master (using a mix of 71 and 72x) does not seem to work either on some zoom. On a wide screen (like 1200px), set the zoom to 75%, and we still see the gap. And on zoom without dev tools (just zoom the regular page with CMD +/-), we also observe the gap on some zoom settings |
On this video, I am scrolling down the page (white content background for easier detection) Loom Message - 4 September 2024 - Watch Video Notice that the gap is appearing only half of the time, the gap is coming from a rounding error |
Got more insight on this issue:
Look at the video (you may need to download and zoom it): as it's scrolling down, the sticky nav is moving up and down by half a pixel on each scroll (which is why we see the gap only half the time). This half pixel up/down move also make the sticky nav off pixel, making the text slightly blurred. Screen.Recording.2024-09-04.at.16.53.42.mp4This is technically a browser issue, and any fix will be hacky |
I think we should prioritise it working on 200% zoom to emulate HiDPI mobile screens, otherwise we would have mobile devices with this issue. Does using 71px achieve that? |
Always using 71px (so losing 1px by overlapping elements) works. This issue technically also happens everywhere we're using 72px, like the sidebar, or sticky nav on other page. It does not happen on the space settings page though, and still trying to figure out why, and if we could reproduce and use this method everywhere. |
@bonustrack do you reproduce 1px gap between topnav and sticky elements on your device here? |
Ideally we should move this logic and style into a component that we can reuse and style without changing multiple views. We could also add gesture slide left and right to move between tabs. |
And the 1px bug is something you can reproduce on Chrome mobile, (Android for me, not sure on iOS). But snapshot.box don't have any 1px bug left I believe. |
Summary
The sticky nav has a different position on narrow screen, making it "jump" by one pixel on scroll
Weirdly, there's 2 different positions (71px and 72px), depending on screen size, even though the topnav always have the same height in all screen
How to test