Problem/Motivation
When you load a new page, the sidebar opens every time. Even though it's fast, it creates a flickering behavior.
Proposed resolution
We can come up with a way to keep the state opened or closed based on the used interaction in the future, but for now for the prototype we can define that the default state is opened.
Issue fork navigation-3375855
Show commands
Start 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:
- 3375855-avoid-sidebar-flickering
changes, plain diff MR !28
- 1.x
changes, plain diff MR !27
Comments
Comment #2
claireristow commentedHey @ckrina, do you mean that we should have the default state be closed for now?
If so, I tackled this in a separate issue (now on the 1.x branch) so I think we can close this one?
Comment #3
ckrinaNo, the default should be opened for now. When you access a page, we should assume the sidebar will be opened, not collapsed. In the future we should come up with a way that doesn't cause any flickering and respects whatever was the last state. But since this will be complicated for the initial implementation of the prototype and we need to choose either opened or closed for now, the default should be opened.
Comment #4
mherchelDuring the meeting today, we talked with Sascha from Gin. They handle this using a combination of localStorage and JS that isn't dependent on DrupalBehaviors. We can go this route.
Comment #5
claireristow commentedCool, I'll start working on this! I'll revert back to a default open state and use localStorage outside of behaviours.
Comment #9
claireristow commentedHey @mherchel or @ckrina, I'm looking for some guidance on this one. I've added drupal behaviours, moved the initial localStorage check outside of the behaviours and I'm still seeing the flash. Any suggestions?
Comment #10
claireristow commentedComment #11
mherchelOK. I played around with this a bit, and there are a couple things:
navigation.html.twigtemplateHowever, not sure how the framework managers feel about this
Comment #12
mherchelDoing a little bit of research and I see that some inline JS was added to remove the current toolbar's flickering, so I'm guessing this is OK, but I'd still like to get a +1.
See #2998451: Toolbar tray rendering can result "flickering" resizing of content area to accommodate open trays
Comment #13
lauriiiInline JS is not ideal but we couldn't come up with anything better in #2998451: Toolbar tray rendering can result "flickering" resizing of content area to accommodate open trays 🤷♂️ Given that it's a pre-existing approach for the toolbar, I think we can assume this will be accepted in core.
Comment #14
ckrina+1 to what @lauriii said.
Comment #15
bnjmnmThe inline JS approach seems like the best (though we acknowledged not ideal) one. It was discovered after its implementation that comments within that JS do not work well in some aggregation scenarios, so be sure to avoid that in your solution.
Comment #16
claireristow commentedThanks for all the input everyone! I'll go with the inline JS.
Comment #18
claireristow commentedMerged!