If a user is on an admin page and the 'escapeAdminPath' sessionStorage is null, the data-toolbar-escape-admin button will flicker and cause a reflow due to its default text of "Back to site" changing to the different-width "Home" after Drupal.behaviors.escapeAdmin completes.
Open an admin page in a new tab or on an existing admin page, delete the 'escapeAdminPath' sessionStorage item and refresh.
Remove the logic that changes the link text so it always says "Back to site", which is accurate link text whether it links to the non-admin home page or the last visited non-admin page. This eliminates the text change that causes the reflow.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | Screen Shot 2023-05-03 at 20.54.14.png | 39.71 KB | lauriii |
| #14 | Screen Shot 2023-05-03 at 20.54.05.png | 40.46 KB | lauriii |
| #6 | Screen Recording 2023-05-02 at 7.57.00 AM.gif | 1.56 MB | bnjmnm |
| #3 | home-button-no-reflow.gif | 317.52 KB | bnjmnm |
| back-to-site-home.gif | 379.63 KB | bnjmnm |
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:
compare
Comments
Comment #3
bnjmnmMR with a solution is up. This sets a minimum width on the button that keeps track of the button's maximum possible width and sets it to that so there's room for the new content to be added without reflowing.
The button contents now fade in as well, so the JS updating of the text ideally isn't visible, but on slower systems it will still be less apparent.
Comment #4
smustgrave commentedConfirmed the "flicker" when I deleted escapeAdminPath in chrome.
MR does solve that.
Not sure about the fade in feature but won't hold the ticket up on a detail like that.
Comment #5
lauriiiI'm not sure I understand why we need the fading here. It is quite notable, potentially even more so than the current reflow. I tried removing the fading from the patch and it seems that that improves the situation without any negative side effects. Moving to needs work to either remove the fade or to document why it's needed.
Comment #6
bnjmnmI didn't like the name switching like that and figured I'd give it a shot at least,
But it's not super important to me and I removed it. A better version would keep the icon in place, too.
I also figured it was something a reviewer might notice if they were paying specific attention to that toolbar tab. However, that text switching is pre-existing AND this issue is specific to addressing the reflow so it's not like it has to happen here even if it was something there was interest in.
Comment #7
bnjmnmNote the gifs in #6 and #3 are playing slower than what I recorded. Anybody basing a decision that is impacted by that slowness should probably evaluate locally.
Comment #8
smustgrave commentedSeems removing add some failures in the tests.
Comment #9
bnjmnmComment #10
smustgrave commentedTested following the same steps in #4.
Don't see the flicker with "back to site" link but do see it with my username. Think that's out of scope but wanted to point out if it matters or not.
Comment #11
lauriiiThis needs a reroll now that #3351356: Reduce toolbar user button related browser reflow landed.
Comment #13
hooroomooRerolled and confirmed that width of the admin escape doesn't change after deleting the 'escapeAdminPath' sessionStorage item and refreshing.
Comment #14
lauriiiJust realized that this change actually leads into before/after changes. When I tested this last time on #5, I didn't realize that the session storage wouldn't be set on the initial page load, and didn't realize there was this change.
Before:
After:
I'm wondering if people actually expect to see the "Home" link, rather than "Back to site". IMO back to site isn't wrong, even if it links to the front page of the site. Maybe we could address this by changing the link text always to "Back to site". Thoughts?
Comment #15
bnjmnmIt definitely crossed my mind that this whole thing would be solved by not changing the link wording at all. I'm skeptical that the distinction between "Home" and "Back to Site" provides much benefit or is even noticed. My biggest reason for the current approach is that it's the way to fix the reflow with the fewest overall changes, and extra-width "home" will appear far less frequently than same-width "back to site"
The change was made 9 yrs ago in #2349569: 'Back to site' link does not work as expected. I wonder if the problem had more to do with the link not consistently being there, vs the wording used. I wouldn't mind just removing that conditional text entirely if there were some +1-ing of that.
Comment #16
lauriiiThank you for linking the original issue where this was introduced @bnjmnm! Based on that, there wasn't any strong reason to use a separate text for the home link, and we would not be re-introducing that bug if we changed the text.
Comment #19
bnjmnmThis is the solution I prefer anyway, glad there's support for it. I opened a
do-not-switch-to-saying-homeMR that eliminates the reflow by not changing that button's text.Comment #20
smustgrave commentedHate to be that guy but since the solution changed can we update the issue summary with the proposed solution please.
Comment #21
bnjmnmNo prob, issue summary updated.
Comment #22
hooroomooWorks like a charm
Comment #23
ckrinaLate to the party, but 100% agree with the late change to always use the same "Back to site" text.
Comment #25
lauriiiCommitted ca3d4fe and pushed to 10.1.x. Thanks!