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

report: avoid paint storms on scrolly header #5207

Merged
merged 6 commits into from
May 21, 2018
Merged

Conversation

paulirish
Copy link
Member

And animate the clouds on hover

Here's the diff on the paint storms. Before and after:

layerslh

Check out the deployment to try the animated clouds.

@paulirish paulirish changed the title report: avoid paint storms on scrolly header. report: avoid paint storms on scrolly header May 15, 2018
</g>
<g mask="url(#b)" fill="#FFF">
<g transform="translate(96 86)">
<circle cx="87.5" cy=".5" r="1" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

these are the little stars right? should we just remove them? they look a little weird, deleting this whole block looks better IMO

before after
image image

|

text-align: right;
transform-origin: bottom right;
will-change: opacity; /* opacity is changed on scroll */
Copy link
Collaborator

Choose a reason for hiding this comment

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

see the score scales look fine with this now, how are you supposed to tell when the text will be messed up by this?

animation: panacross 30s linear infinite;
animation-play-state: paused;
}
.lh-header-sticky:hover .lh-lighthouse__clouds {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the :hover threw me for a while since the hitbox includes the dropdown of the scores, how much do you like the hover effect vs. always? :)

Copy link
Member

Choose a reason for hiding this comment

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

I'm personally ok with always, too :)

Something something lighthouse is always looking out for you metaphor

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

this wfm, animation is great! can't wait to multiply those clouds :D

☁️ ☁️ ☁️ ☁️ ☁️ ☁️ ☁️ ☁️ ☁️ ☁️

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

LGTM with blurry URL fix

👍 👍 on the var renames, too

animation: panacross 30s linear infinite;
animation-play-state: paused;
}
.lh-header-sticky:hover .lh-lighthouse__clouds {
Copy link
Member

Choose a reason for hiding this comment

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

I'm personally ok with always, too :)

Something something lighthouse is always looking out for you metaphor

@brendankenny
Copy link
Member

whats the deal

@paulirish paulirish changed the title report: avoid paint storms on scrolly header report: avoid paint storms on scrolly header. May 21, 2018
@paulirish paulirish requested a deployment to PR staging May 21, 2018 16:58 Pending
@paulirish paulirish changed the title report: avoid paint storms on scrolly header. report: avoid paint storms on scrolly header May 21, 2018
@paulirish paulirish merged commit 804e9b8 into master May 21, 2018
@paulirish paulirish deleted the headertransforms branch May 21, 2018 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants