Investigate better ways to add anti-flicker JS

Issue created by @bnjmnm

Comment about 1 year ago →

🇿🇦South Africa stewest Cape Town

We're noticing that the comments from Line 68 web/core/modules/toolbar/toolbar.module are showing when logged out.

Comment about 1 year ago →

🇺🇸United States kevinquillen

Same as #3. This should only load for appropriate users.

Comment 12 months ago →

🇺🇸United States drewcking

Is there a need for these comments to be in the javascript? It makes sense to move them above this heredoc so that they are PHP comments instead of JS comments.

Comment 12 months ago →

🇺🇸United States bnjmnm Ann Arbor, MI

Is there a need for these comments to be in the javascript? It makes sense to move them above this heredoc so that they are PHP comments instead of JS comments.

Good call. No need. I think the comments presence in the heredoc was due to that block of code originally being in a JS file and we didn't think to adjust for it going into the heredoc.

Comment 12 months ago →

🇰🇪Kenya fngatia Eldoret

Am also having problems with the comments as indicated above. My custom js are injecting data/code into the tag in the comments instead of the HTML one. Can the tags wrap tag be avoided?

```

(function() { const toolbarState = sessionStorage.getItem('Drupal.toolbar.toolbarState') ? JSON.parse(sessionStorage.getItem('Drupal.toolbar.toolbarState')) : false; // These are classes that toolbar typically adds to //

, but this code
// executes before the first paint, when is not yet present. The
// classes are added to so styling immediately reflects the current
// toolbar state. The classes are removed after the toolbar completes
// initialization.
const classesToAdd = ['toolbar-loading', 'toolbar-anti-flicker'];
if (toolbarState) {
const {
orientation,
hasActiveTab,
isFixed,
activeTray,
activeTabId,
isOriented,
userButtonMinWidth
} = toolbarState;

classesToAdd.push(
orientation ? `toolbar-` + orientation + `` : 'toolbar-horizontal',
);
if (hasActiveTab !== false) {
classesToAdd.push('toolbar-tray-open');
}
if (isFixed) {
classesToAdd.push('toolbar-fixed');
}
if (isOriented) {
classesToAdd.push('toolbar-oriented');
}

if (activeTray) {
// These styles are added so the active tab/tray styles are present
// immediately instead of "flickering" on as the toolbar initializes. In
// instances where a tray is lazy loaded, these styles facilitate the
// lazy loaded tray appearing gracefully and without reflow.
const styleContent = `
.toolbar-loading #` + activeTabId + ` {
background-image: linear-gradient(rgba(255, 255, 255, 0.25) 20%, transparent 200%);
}
.toolbar-loading #` + activeTabId + `-tray {
display: block; box-shadow: -1px 0 5px 2px rgb(0 0 0 / 33%);
border-right: 1px solid #aaa; background-color: #f5f5f5;
z-index: 0;
}
.toolbar-loading.toolbar-vertical.toolbar-tray-open #` + activeTabId + `-tray {
width: 15rem; height: 100vh;
}
.toolbar-loading.toolbar-horizontal :not(#` + activeTray + `) > .toolbar-lining {opacity: 0}`;

const style = document.createElement('style');
style.textContent = styleContent;
style.setAttribute('data-toolbar-anti-flicker-loading', true);
document.querySelector('head').appendChild(style);

if (userButtonMinWidth) {
const userButtonStyle = document.createElement('style');
userButtonStyle.textContent = `#toolbar-item-user {min-width: ` + userButtonMinWidth +`px;}`
document.querySelector('head').appendChild(userButtonStyle);
}
}
}
document.querySelector('html').classList.add(...classesToAdd);
})();

```

Comment 12 months ago →

🇨🇦Canada Ambient.Impact Toronto

I'm a bit baffled as to why this made it into a stable release because it feels like this is not a good way to handle this. Core intentionally moved away from inline JavaScript years ago, and this is non-usable on sites with Content Security Policy 📌 Disable toolbar anti-flicker inline JS Fixed set up. Just putting this into render-blocking external JavaScript and CSS files linked from the <head> should accomplish the same thing, in that browsers should prioritize downloading that before starting to render the page, should it not?

Comment 11 months ago →

🇦🇷Argentina lucasvm

I can confirm this issue still exists on Drupal 10.1 i got fixed by adding this to the .theme file instead of creating a new page inc

function THEME_page_attachments_alter(&$variables) {
  if(\Drupal::currentUser()->isAnonymous()) {
    $variables['#attached']['html_head'] = array_filter($variables['#attached']['html_head'], function($item) {
      return $item[1] !== 'anti_flicker_js';
    });
  }

Comment 11 months ago →

🇺🇸United States bnjmnm Ann Arbor, MI

I'm a bit baffled as to why this made it into a stable release because it feels like this is not a good way to handle this

The reasons why it was not done this way was discussed in the issue where it was implemented 🐛 Toolbar tray rendering can result "flickering" resizing of content area to accommodate open trays Fixed . There's was also an acknowledgement that it might not be the ideal approach, thus the existence of this issue.

I look forward to seeing MRs or patches that improve it, as there seems to be enough interest in this to result in some motivation.

Comment 11 months ago →

🇺🇸United States Danny Englander San Diego

I just got hit with this one. My use case is, I have Laveral Mix and BrowserSync running which injects some inline JS to hot reload the page when the CSS has changed from my Sass build. The JS from Laveral Mix inserted itself right within the anti-flicker script so as to break the page just like the screen capture in #3.

Comment 11 months ago →

🇬🇧United Kingdom catch

I just found 🐛 Toolbar anti-flicker js shows for anonymous users Closed: duplicate and in the process of searching for duplicates found this issue.

I wasn't involved in the previous issue, but I think we should do;

Having this JS be part of a library with header: true, which was decided against as it adds an additional JS request, even when assets are aggregated

Once 🐛 Toolbar anti-flicker js shows for anonymous users Closed: duplicate is fixed we'd only be adding the JavaScript for users with access to the toolbar, these are authenticated with admin or semi-admin permissions who will mostly be browsing the site with the file already browser cached. So for me it is not worth the workaround to save the cold cache http request for those users.

The other reason is that we already have a library with render-blocking js in the header, although I opened an issue to remove that here: 📌 Re-evaluate touchevents detection Needs work . But until that's done it won't be an extra http request in most cases.

Investigate better ways to add anti-flicker JS

Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments & Activities