search

PaperMC / velocitypowered.com

The revamped Velocity website built on Gatsby
https://velocitypowered.com
MIT License
5 stars 13 forks source link

feat: transition on theme change #30

Closed VottusCode closed 3 years ago

hugmanrique commented 3 years ago

Adding a transition to every single element on a page for all property changes will break the site's performance. You could add the transition manually on the theme switcher component, but imo this overcomplicates things for such a small visual change.

VottusCode commented 3 years ago

I don't think the performance is a problem in this case, or am I wrong? I never had a performance problem with such thing? Of course i could add it for each component, but that, as you said, overcomplicates things for such a small change.

hugmanrique commented 3 years ago

Not on a relatively modern computer, but mobile users will suffer. Either way, this unintentionally adds e.g. hover and focus color transitions to links.

astei commented 3 years ago

I don't have statistics, but I doubt most people browsing the Velocity website are on mobile devices. I believe desktop devices probably make up the majority of people browsing our website.

VottusCode commented 3 years ago

Not on a relatively modern computer, but mobile users will suffer. Either way, this unintentionally adds e.g. hover and focus color transitions to links.

Is that necessarily a bad thing?

astei commented 3 years ago

Let's try it.

astei commented 3 years ago

This works fine on Firefox but it's catastrophically broken on Chrome and WebKit/Safari. Reverting.

VottusCode commented 3 years ago

Sorry. Somehow I missed the pull request. How is it broken? // Edit: Seems like on Chrome/Safari some parts of the website change after the rest of it. Gonna take a look at it.