Skip to content
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

Improve image loading behaviors #4295

Merged
merged 5 commits into from
May 20, 2020

Conversation

Munter
Copy link
Contributor

@Munter Munter commented May 19, 2020

I looked through what we are doing with loading opencollective avatars and found some improvements.

  • Sponsor logos now have dimensions assigned to avoid page reflows as they load
  • Removed the behavior where we'd wait for all logos to load before showing any of them
  • Logos now fade in individually, starting out with a gray background
  • Sponsor logos and backer avatars now also show with javascript disabled
  • Moved backers and sponsors into the same file and put the javascript into it as well, so it's easier to see what the script is working on.
  • Inlined the avatar javascript so the loading behavior is optimized
  • Set lazyload on all other content images

@Munter Munter requested a review from boneskull May 19, 2020 23:09
@Munter Munter changed the title Munter/show all sponsors Improve image loading behaviors May 19, 2020
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.228% when pulling d492b62 on munter/show-all-sponsors into 76f2142 on boneskull/show-all-sponsors.

await Promise.all(
supporters.sponsors.map(async sponsor => {
for await (const chunk of needle.get(sponsor.avatar)) {
sponsor.dimensions = imageSize(chunk);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. yeah, they come down with a fixed height, but we don't know the width.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. HAving the width makes all the difference for being able to have the browser finalize the height of the section because it knows what space to reserve

</ul>

<script>
(function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this still gets squashed in production.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'll just need to check up on the bundling there. I'd want this one to actually stay in place and inlined, which gives us the least amount of lag time between rendering the things it works on and initiating the effect it creates

@boneskull boneskull merged commit cb74950 into boneskull/show-all-sponsors May 20, 2020
@Munter Munter deleted the munter/show-all-sponsors branch May 20, 2020 19:05
@boneskull
Copy link
Contributor

feel free to push again to my branch

@boneskull
Copy link
Contributor

rather, directly, no need to PR

@craigtaub craigtaub added this to the next milestone May 31, 2020
@craigtaub craigtaub added the area: website involving mochajs.org, but not necessarily involving docs label May 31, 2020
@boneskull boneskull added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Jun 1, 2020
@boneskull boneskull modified the milestones: next, v8.0.0 Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: website involving mochajs.org, but not necessarily involving docs semver-patch implementation requires increase of "patch" version number; "bug fixes"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants