I cache is disabled, every time the page loads the items on the Navigation flickr briefly (text disappears). It all happens in milliseconds, but it's perceptible and it makes it feel the Navigation as not stable or "not finished".
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | After-patch.mp4 | 1.18 MB | kanchan bhogade |
| #8 | Before-patch.mp4 | 191.88 KB | kanchan bhogade |
| flickering.gif | 355.07 KB | ckrina |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
ckrinaComment #3
KeyboardCowboyComment #4
finnsky commentedThis is controlled here.
https://git.drupalcode.org/project/navigation/-/blob/1.x/css/components/...
It was added to:
When label is `collapse-sidebar` and localstorage setting is collapsed then we hide label until js loaded.
We need to add it only for buttons which are sidebar collapse/expand triggers.
Comment #6
finnsky commentedAdded few optimisations.
Top level menu items looks much better now.
Problem still exists with child menus.
probably we need to move this from js `if (document.URL === link.href) {` to some PHP trick?
Work still needed here.
Comment #7
finnsky commentedRemoved behaviours usage. now works faster. Please review
Comment #8
kanchan bhogade commentedHi
I've tested MR !187 on Drupal 11.x
Test Result: No flickering effect after adding the patch
Adding videos for reference
Keeping in "needs review" for code verification and more reviews
Comment #9
Sanjeev P commentedReviewed the issue. Now the sidebar reloads faster and no flicker found. It looks fixed.
Can be moved to RTBC++
Keeping in Needs Review for further review.
Comment #10
ckrinaMoving this to NW to get visibility on my comments about the font rendering and font weight changes.
Comment #11
finnsky commentedComment #15
starshapedLooks good to me, tested in Firefox, Chrome, and Safari. The JS looks good to me too, always nice to shave off a few ms!
Comment #16
kostyashupenkoAdded comment in MR, please check
Comment #17
finnsky commentedI would better reopen follow up for this. Because this MR contains more than this font-display.
Comment #18
ckrinaI just opened #3432097: Decide font-display strategy to leave the conversation about the font strategy out of this so we don't block the improvements this is introducing.
I've just tested this and found the same error, that doesn't happen in
1.x, so the regression is being introduced here. So leaving this as to Needs work to fix that.Comment #19
finnsky commentedit happends because of big-pipe replacement.
shortcut link is initially just link. and child items attached later. but when i removed behaviours this big-pipe replacement happends but no behaviours attached.
you can disable behaviours and check
so question is. revert behaviours or disable big-pipe for navigation somehow?
Comment #20
finnsky commentedI've added reattach with behaviours. it works but not sure about it.
Comment #21
finnsky commentedCleaned this a bit more. Now only Big Pipe replaced items became problem. But works.
Comment #23
finnsky commentedAlright added extra care about behaviours in case of BigPipe replacements. It is ready to go.
Comment #24
ckrinaMerged, thanks everybody! This is a great improvement 🎉