Reduce reflow on the Home / Back to Site button.

Created on 28 April 2023, over 1 year ago
Updated 5 May 2023, over 1 year ago

Problem/Motivation

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.

Steps to reproduce

Open an admin page in a new tab or on an existing admin page, delete the 'escapeAdminPath' sessionStorage item and refresh.

Proposed resolution

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.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Fixed

Version

10.1

Component
Toolbar 

Last updated about 1 month ago

  • Maintained by
  • 🇫🇷France @nod_
Created by

🇺🇸United States bnjmnm Ann Arbor, MI

Live updates comments and jobs are added and updated live.
  • Field UX

    Usability improvements related to the Field UI

Sign in to follow issues

Comments & Activities

  • Issue created by @bnjmnm
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,366 pass
  • @bnjmnm opened merge request.
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    MR 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.

  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States smustgrave

    Confirmed 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.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,366 pass
  • Status changed to Needs work over 1 year ago
  • 🇫🇮Finland lauriii Finland

    I'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.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Build Successful
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    I'm not sure I understand why we need the fading here

    I 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.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Note 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.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Seems removing add some failures in the tests.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,371 pass
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI
  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States smustgrave

    Tested 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.

  • Status changed to Needs work over 1 year ago
  • 🇫🇮Finland lauriii Finland

    This needs a reroll now that 📌 Reduce toolbar user button related browser reflow Fixed landed.

  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,374 pass
  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States hooroomoo

    Rerolled and confirmed that width of the admin escape doesn't change after deleting the 'escapeAdminPath' sessionStorage item and refreshing.

  • Status changed to Needs review over 1 year ago
  • 🇫🇮Finland lauriii Finland

    Just 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?

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    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?

    It 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.

  • 🇫🇮Finland lauriii Finland

    Thank 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.

  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Not currently mergeable.
  • @bnjmnm opened merge request.
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    This is the solution I prefer anyway, glad there's support for it. I opened a do-not-switch-to-saying-home MR that eliminates the reflow by not changing that button's text.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,378 pass
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Hate to be that guy but since the solution changed can we update the issue summary with the proposed solution please.

  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    No prob, issue summary updated.

  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States hooroomoo

    Works like a charm

  • 🇪🇸Spain ckrina Barcelona

    Late to the party, but 100% agree with the late change to always use the same "Back to site" text.

    • lauriii committed ca3d4fe5 on 10.1.x
      Issue #3357112 by bnjmnm, hooroomoo, lauriii, smustgrave, ckrina: Reduce...
  • 🇫🇮Finland lauriii Finland

    Committed ca3d4fe and pushed to 10.1.x. Thanks!

  • Status changed to Fixed over 1 year ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024