New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add delay to deleting Critical CSS when all is cached #1073

Merged
merged 2 commits into from May 24, 2017

Conversation

Projects
None yet
3 participants
@tlemburg
Contributor

tlemburg commented May 23, 2017

When doing a hard refresh on speedy template, there
is a flash of NO CSS. This occurs because the javascript
quickly throws in the deferred CSS to be loaded from cache,
then immediately deletes the critical CSS, which leaves no CSS
until the deferred CSS is loaded.

This change adds a 1 second delay before removing this CSS. This
should be enough time for almost all computers and phones to get
their stylesheet, parse, and render it to avoid this flash of no CSS.

I looked at simply not deleting the critical CSS, but that was a bigger
change that didn't need to be made at this time and without understanding
everything that is placed in each part, it's probably not a great
idea to tinker with it at this time.

Tyler Lemburg
Add delay to deleting Critical CSS when all is cached
When doing a hard refresh on speedy template, there
is a flash of NO CSS. This occurs because the javascript
quickly throws in the deferred CSS to be loaded from cache,
then immediately deletes the critical CSS, which leaves no CSS
until the deferred CSS is loaded.

This change adds a 1 second delay before removing this CSS. This
should be enough time for almost all computers and phones to get
their stylesheet, parse, and render it to avoid this flash of no CSS.

I looked at simply not deleting the critical CSS, but that was a bigger
change that didn't need to be made at this time and without understanding
everything that is placed in each part, it's probably not a great
idea to tinker with it at this time.
@mfairchild365

This comment has been minimized.

Show comment
Hide comment
@mfairchild365

mfairchild365 May 23, 2017

Member

This might help, but there is no guarantee that the the deferred CSS will be loaded after 1 second, especially on slower connections. Why not use a callback?

Member

mfairchild365 commented May 23, 2017

This might help, but there is no guarantee that the the deferred CSS will be loaded after 1 second, especially on slower connections. Why not use a callback?

@tlemburg

This comment has been minimized.

Show comment
Hide comment
@tlemburg

tlemburg May 23, 2017

Contributor

I didn't want to use loadCSS here because that would put the stylesheet at a different spot than in the head tag: "The code above will insert a new CSS stylesheet link after the last stylesheet or script that it finds in the page". I just realized though that it actually does the same thing. I also kinda forgot that if we were hard-refreshing, you wouldn't be pulling the all.css from the cache at all. So the description above doesn't make much sense.

So I think the better plan then is to totally just forget about the sessionStorage thing, since it really doesn't do anything

Contributor

tlemburg commented May 23, 2017

I didn't want to use loadCSS here because that would put the stylesheet at a different spot than in the head tag: "The code above will insert a new CSS stylesheet link after the last stylesheet or script that it finds in the page". I just realized though that it actually does the same thing. I also kinda forgot that if we were hard-refreshing, you wouldn't be pulling the all.css from the cache at all. So the description above doesn't make much sense.

So I think the better plan then is to totally just forget about the sessionStorage thing, since it really doesn't do anything

Tyler Lemburg
Change process to not use sessionStorage to determine cache state
And always use loadCSS to load the stylesheet.
sheet.href = 'https://unlcms.unl.edu/wdn/templates_4.1/css/all.css?dep=$DEP_VERSION$';
document.getElementsByTagName('head')[0].appendChild(sheet);
};
var WDNRemoveCriticalStyles = function() {

This comment has been minimized.

@mfairchild365

mfairchild365 May 23, 2017

Member

Now that this is only being used in one spot, it doesn't make much sense for it to be its own function.

@mfairchild365

mfairchild365 May 23, 2017

Member

Now that this is only being used in one spot, it doesn't make much sense for it to be its own function.

@tlemburg tlemburg merged commit 99558d7 into unl:develop May 24, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tlemburg tlemburg deleted the tlemburg:FOnoCSS_fix branch May 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment