Optimize homepage #15980

wjordan · 2017-06-20T21:41:37Z

This PR makes various small tweaks/updates to the homepage. Half of the changes are helpful in preloading the initial hero-image (moving critical content to appear as early as possible in the loading sequence), the other half are some general cleanup.

Preload first hero image

~~preload first hero image using a preload link tag~~
~~select _2x.jpg image based on min-width: 1440px instead of devicePixelRatio (prevent non-2x images from being zoomed in too close)~~
~~use CSS media queries instead of rewriting URLs in JavaScript (necessary for preload link)~~
~~use CSS animation to fade-in first hero image (better rendering performance, and it can begin fading in immediately before jQuery is loaded)~~
~~use lazysizes bgset to lazy-load subsequent hero images~~

Other cleanup

remove jquery.placeholder polyfill- no longer needed for modern browsers
lazy-load a few more images that may never be rendered (language flag, gallery thumbnails)
remove ajax requests made obsolete by page/header redesigns (~~user_menu~~, user_hero).
Add explicit width/height dimensions to some elements (eliminates jarring/time-consuming page reflow after images are loaded)
~~Move donor-link shuffle to client-side javascript (when HTML is cached, shuffling content in server code doesn't work as expected).~~

Demo

https://adhoc-optimize-homepage.cdn-code.org/

joshlory · 2017-06-20T21:54:52Z

pegasus/sites.v3/code.org/views/homepage_hero.haml

+          #fullwidth#{index} {
+            background-image: url(#{entry[:image_t]});
+          }
+        }


Can you move this inline CSS to the appropriate .css file?

I realize that you have it inlined here to generate the correct URLs for each image. I'd really prefer we use the srcset attribute instead: https://stackoverflow.com/a/28731347.

This CSS style is dynamically generated based on server-side inputs, so can't be included in a static file.

srcset attribute is for the img element, whereas these images are set with a background-image CSS style, which is a different system that I don't think are compatible. There is an image-set() CSS notation, but it's still just a draft with limited support, and doesn't (yet) support width-based sets (just resolution-based).

joshlory · 2017-06-20T21:58:29Z

pegasus/sites.v3/code.org/views/petition_expand.haml

@@ -82,9 +82,6 @@

  function Petition()
  {
-    // placeholder text for non-HTML5 browsers
-    $('input[type=text], textarea').placeholder();
-    $('input[type=email], textarea').placeholder();


Not needed? Can this whole function go away (and wherever we call it)?

the jquery.placeholder polyfill is no longer needed for modern browsers.

petition = new Petition(); and petition.signPetition can't go away, so I'm pretty sure the empty function definition needs to remain in order for the JS prototype-class to work (I could be wrong about this though)

joshlory · 2017-06-20T21:59:07Z

pegasus/sites.v3/code.org/views/donor_slider.haml

+          // Shuffle donor-slider links.
+          var ul = document.querySelector('.donor-slider');
+          for (var i = ul.children.length; i >= 0; i--) {
+            ul.appendChild(ul.children[Math.random() * i | 0]);


Bitwise or (|) intentional? Or should this be ||?

The bitwise-or truncates a floating-point, to generate a random index-integer from 0 to i-1.

A comment to explain this trick would be handy for newcomers.

joshlory · 2017-06-20T21:59:36Z

pegasus/sites.v3/code.org/views/donor_slider.haml

@@ -21,3 +20,9 @@
            = "#{supporter[:name_s]}"
          - else
            != "#{supporter[:name_s]}, "
+        :javascript


Please move this inline JS to an appropriate .js file.

Linking external resources pauses page-loading and would negatively impact performance.

See: Render-Blocking JavaScript

breville · 2017-06-21T01:31:43Z

re: "select _2x.jpg image based on min-width: 1440px instead of devicePixelRatio (prevent non-2x images from being zoomed in too close)"

Does that mean we no longer load retina images for retina devices with a display narrower than 1440px, such as tablets and phones?

breville · 2017-06-21T01:34:45Z

If you visit the page for the first time (or with caching disabled) the second image in the banner rotation isn't available when the first image starts fading out, so you see the first image fade to white before the next image suddenly appears. The second image should be loaded by the time the first image is fading out so that there is a nice cross-fade.

wjordan · 2017-06-21T01:39:56Z

@breville fwiw, the fading-to-white in between images (it's more noticeable on a low-bandwidth connection) shouldn't be new to this PR, this behavior would have been introduced by #15405.

It will take a bit more work, but should be possible to add logic to fully pre-load the next image in the carousel before initiating the cross-fade.

wjordan · 2017-06-21T02:00:26Z

Does that mean we no longer load retina images for retina devices with a display narrower than 1440px, such as tablets and phones?

Correct, this is the intended change. The difference is that the device's pixel ratio doesn't matter as much as how many pixels the device actually requires to render the image. This will prevent too many pixels from being sent to high-dpi but low-total-pixel phones, and vice versa, will prevent too few pixels from being sent to normal-dpi but high-total-pixel monitors. This is the current best practice, from what I've been able to find- see Screen Width Optimization Using srcset for some discussion + visual aids.

breville · 2017-06-21T02:09:34Z

Good point. That does make sense then.

wjordan · 2017-06-21T16:38:18Z

@joshlory and @breville thanks both for the detailed feedback! I've decided to better organize the diffs by extracting two separate, more self-contained PRs- one dealing with the hero-images and related CSS, and another dealing with the donor-link shuffling JS, leaving the other (more minor) improvements in this PR. Will add links to both when they're ready.

wjordan · 2017-06-21T16:56:40Z

pegasus/sites.v3/code.org/public/index.haml

-        }
-      }
-    });
-  });


This ajax call can be removed (and the #petitionexpand div initially set to display: block) now that users are no longer directed to the homepage when logged-in. /dashboardapi/user_hero is implemented with head :not_found unless current_user, so will now always return :not_found, so we can skip this ajax step and always start the page with petition block expanded.

To be clear, the petition should visually still not be expanded.. we still want the user to click "I agree" before expanding to show the whole form. But it makes sense to show the unexpanded petition right away.

right, the petitionexpand div will be visible (but not expanded) on initial load, the contained form div will not be 'expanded' (via expandPetition()) until user action.

wjordan · 2017-06-21T18:52:29Z

Finished extracting the hero-image preload into #16002, and removed the donor-link shuffle change for now. I think the remaining items are ready to merge.

- remove jquery.placeholder - lazy-load more images (language flag, gallery thumbnails) - remove unnecessary ajax requests - set image dimensions on elements to avoid reflow on image-loads.

breville · 2017-06-21T20:55:30Z

Just a second thought about using min-width to decide whether to show 2x images or not... doesn't iOS Safari report width/height of 1024x768 on iPad, for example, whether retina or not? I don't think it actually tells us how many pixels there are to display. http://stephen.io/mediaqueries/

wjordan · 2017-06-21T21:26:22Z

yeah, I think you might be right - the px in min-width: 1440px is (incorrectly) css-pixels while the w in 1440w is (correctly) 'device-pixels', so the width-based media query wouldn't work as I originally described it above. The extracted PR #16002 didn't implement any width-based media query yet (I'll leave that as a future todo) so should be unaffected for now.

wjordan requested a review from breville June 20, 2017 21:41

joshlory reviewed Jun 20, 2017

View reviewed changes

wjordan force-pushed the optimize-homepage branch from e59e6e3 to cbedb3b Compare June 21, 2017 16:54

wjordan commented Jun 21, 2017

View reviewed changes

wjordan mentioned this pull request Jun 21, 2017

Preload first hero image on homepage #16002

Merged

wjordan force-pushed the optimize-homepage branch from 455d028 to 4fc809f Compare June 21, 2017 19:05

Optimize homepage

dcc3cac

- remove jquery.placeholder - lazy-load more images (language flag, gallery thumbnails) - remove unnecessary ajax requests - set image dimensions on elements to avoid reflow on image-loads.

wjordan force-pushed the optimize-homepage branch from 4fc809f to dcc3cac Compare June 21, 2017 20:14

wjordan merged commit bacc29b into staging Jun 21, 2017

wjordan deleted the optimize-homepage branch June 21, 2017 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize homepage #15980

Optimize homepage #15980

wjordan commented Jun 20, 2017 •

edited

joshlory Jun 20, 2017

joshlory Jun 20, 2017

wjordan Jun 20, 2017

joshlory Jun 20, 2017

wjordan Jun 20, 2017

joshlory Jun 20, 2017

wjordan Jun 20, 2017

breville Jun 21, 2017

joshlory Jun 20, 2017

wjordan Jun 20, 2017 •

edited

breville commented Jun 21, 2017

breville commented Jun 21, 2017

wjordan commented Jun 21, 2017

wjordan commented Jun 21, 2017

breville commented Jun 21, 2017

wjordan commented Jun 21, 2017

wjordan Jun 21, 2017

breville Jun 21, 2017

wjordan Jun 21, 2017

wjordan commented Jun 21, 2017

breville commented Jun 21, 2017 •

edited

wjordan commented Jun 21, 2017

Optimize homepage #15980

Optimize homepage #15980

Conversation

wjordan commented Jun 20, 2017 • edited

Preload first hero image

Other cleanup

Demo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjordan Jun 20, 2017 • edited

Choose a reason for hiding this comment

breville commented Jun 21, 2017

breville commented Jun 21, 2017

wjordan commented Jun 21, 2017

wjordan commented Jun 21, 2017

breville commented Jun 21, 2017

wjordan commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjordan commented Jun 21, 2017

breville commented Jun 21, 2017 • edited

wjordan commented Jun 21, 2017

wjordan commented Jun 20, 2017 •

edited

wjordan Jun 20, 2017 •

edited

breville commented Jun 21, 2017 •

edited