search

pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community
https://pydata-sphinx-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
565 stars 304 forks source link

ENH animation for the top banner #1693

Closed Charlie-XIAO closed 5 months ago

Charlie-XIAO commented 5 months ago

Fixes #1692.

Please feel free to close if this is not a desired feature.

Also I'm not sure if there is a simpler approach. I tried to use translateY but with that approach though the banner will appear gradually the space will be left out without transition which is not desired.

Charlie-XIAO commented 5 months ago

Thanks for spotting those @drammock. Indeed I forgot about the JS for the version warning banner. As for the height problem I can resolve by unsetting the styles and let CSS take over. I've already pushed the changes and here is a slowed-down version of animation (3s transition) to view things more clearly:

https://github.com/pydata/pydata-sphinx-theme/assets/108576690/a26ecf89-3190-4e7b-a33c-709121dc3bb9

12rambau commented 5 months ago

just jumping to say that it is not a "wanted feature" but instead a very nice surprise! what happens if the JS cannot be executed?

choldgraf commented 5 months ago

This isn't a strongly held opinion, but what if we had the banner "pulse" in size or color, but start and end with the same size. That way the pulse could grab people's attention, but with less movement in general (which may feel disruptive to some) and with a starting point that is the ending point so it still appears "normal" if the JS doesn't execute.

This could also make the experience more consistent for people that intentionally disable motion on web pages, which is an accessibility guideline

drammock commented 5 months ago

what if we had the banner "pulse" in size or color, but start and end with the same size. That way the pulse could grab people's attention, but with less movement in general

Currently there is (unwanted) movement, but it is jerky (due to unavoidable JS loading delays). So I think this is an unqualified improvement.

what happens if the JS cannot be executed?

I think without JS, the banner just won't show up at all? IIRC that's how it was set up, but I haven't verified recently.

choldgraf commented 5 months ago

Currently there is (unwanted) movement

You're right! I forgot about that. I just tried it on a "Slow 3G network" throttle and it is nicer than having it just pop out of nowhere. I'm +1 on merging as-is.

Charlie-XIAO commented 5 months ago

Thanks @drammock for the review! I've addressed the comments and applied also to the announcement part.

drammock commented 5 months ago

thanks @Charlie-XIAO !