Enhance main navbar to improve bouncing on macOS#8727
Conversation
|
@vfonic can't tell if this will be merged, but if you want to take a look to this branch |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8727 +/- ##
=======================================
Coverage 99.11% 99.11%
=======================================
Files 141 141
Lines 4074 4074
=======================================
Hits 4038 4038
Misses 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <body class="bg-white dark:bg-gray-950/95 text-gray-950 dark:text-gray-100 antialiased"> | ||
| <%= render "active_admin/site_header", title: site_title %> | ||
| <div class="xl:ms-60"> | ||
| <div class="xl:ms-60 pt-16"> |
There was a problem hiding this comment.
Used pt-16 instead of mt-16 because of Flowbite's documentation, copilot suggestion, and tailwind's website:
|
Wow, that was fast! Works great on both Chrome and Safari. Thanks @tagliala! 🚀 |
javierjulio
left a comment
javierjulio
left a comment
There was a problem hiding this comment.
This breaks the see-through effect. If it's causing issues elsewhere not worth the hassle. Also I do remember copying that style from Tailwind which as you shared from their source they don't use sticky anymore. They did at one point as I confirmed through Web Archive.
There was a problem hiding this comment.
We should avoid making this element change. No need to modify or match what Flowbite is doing in this case and rather not make a change here unless necessary.
There was a problem hiding this comment.
I just wanted to point out that Chrome and other Chromium-based browsers (like Edge, Brave, Arc) make up around 60-70% of desktop browser market share. Since this issue appears in those browsers (I tested on Brave as well), it could affect a large portion of end users.
Safari holds about 15-20% of desktop browser market share.
Since this is an open-source project, prioritizing the fixes that benefit the majority of end-users and developers might be the best approach.
Maybe there's a way to have this fixed so that the see-through effect isn't lost on Safari? I'll see if I can find some win-win solution.
There was a problem hiding this comment.
I just tested this in the latest stable versions of Brave and Safari (Version 18.5 (20621.2.5.11.8)).
To me both, AAv4 beta15 and these changes work the same in Safari: The main page content goes under the translucent header on scroll down. Is this what you mean by "see-through effect"?
Screenshot
In Brave, these changes fix the issue I reported in #8726
Here's the before (AAv4 beta15) screen recording (Safari then Brave):
Screen.Recording.2025-06-01.at.16.24.49.mov
Here's the screen recording after applying this PR's changes (Safari then Brave):
Screen.Recording.2025-06-01.at.16.22.09.mov
NOTE: All the emails in these screenshots/videos are from FFaker gem.
There was a problem hiding this comment.
@vfonic I'm not sure why you commented on this thread. It's not related to my comment. I was only requesting to remove the div->aside change as it's unnecessary. Everything else would remain. That may be a source for confusion as to why this was closed.
|
Got it. This is a minor issue, I'm going to close here for the moment |
|
@tagliala sorry, I think there is confusion coming from my review comment. That was a request to only remove the div->aside change which is unnecessary. Everything else would remain. |
|
Ah sorry, will fix |
tagliala
force-pushed
the
bugfix/8726-fix-navbar
branch
2 times, most recently
from
1f33a5d to
02e27ba
Compare
|
Rebased and fixed. I've added to the commit message why |
|
@javierjulio Sorry, my bad. Something feels slightly different about the GitHub UI. Or maybe I simply didn't see that your comment only related to the div => aside change... |
didn't see that either, but now I've double checked the see-through effect and it looks it's working as before |
tagliala
force-pushed
the
bugfix/8726-fix-navbar
branch
from
02e27ba to
f883bf0
Compare
As specified in the Flowbite documentation, the sticky navbar should utilize the `fixed` class instead of the `sticky` class. This was causing a poor bouncing effect on macOS on all browser except Safari. This commit: - Replaces the `sticky` class with the `fixed` class, to improve bouncing effect - Adds a top padding to the main body element, to take into consideration the space used by the fixed navbar NOTE: Flowbite documentation suggests to use the `aside` tag instead for the sidebar, but this breaks the see-through effect, so `div` is being preserved Close #8726 Ref: - https://flowbite.com/docs/components/sidebar/#sidebar-with-navbar
tagliala
force-pushed
the
bugfix/8726-fix-navbar
branch
from
f883bf0 to
1938b3b
Compare
|
This is potentially a breaking change. I had to update my templates from v15 to v16 |
Explicitly mention breaking changes in the upgrading guide Ref: #8727
3 participants
As specified in the Flowbite documentation, the sticky navbar should
utilize the
fixedclass instead of thestickyclass.This was causing a poor bouncing effect on macOS on all browser except
Safari.
This commit:
stickyclass with thefixedclass, to improvebouncing effect
consideration the space used by the fixed navbar
NOTE: Flowbite documentation suggests to use the
asidetag insteadfor the sidebar, but this breaks the see-through effect, so
divisbeing preserved
Close #8726
Ref: