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

Add animations to the sidebar and other small places #210

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Wulian233
Copy link

@Wulian233 Wulian233 commented Dec 20, 2024

I hope this theme isn't too stiff, let's make it more visually appealing :)

Added animations for showing/hiding the sidebar
Color transition for sidebar buttons

It shouldn't affect normal usage, but it will make it more user-friendly👀

bandicam.2024-12-27.20-06-40-739.mp4

The video has been compressed, so the animation might not look very smooth.

@Wulian233 Wulian233 marked this pull request as draft December 20, 2024 23:23
@Wulian233 Wulian233 changed the title Improve sidebar responsiveness and resizability Add animations to the sidebar and other small places Dec 21, 2024
@Wulian233 Wulian233 marked this pull request as ready for review December 21, 2024 03:00
Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
It seems that this includes 3 changes:

  • The smooth scrolling -- this looks ok to me.
  • The sidebar animation -- from the video it looks like it has some issues, with the gray button lagging behind the rest when it's expanded. If this is fixed it might be ok to add it.
  • A fade-in animation on the page -- I haven't seen this in action, but it seems somewhat unnecessary.

@Wulian233
Copy link
Author

  • The sidebar animation -- from the video it looks like it has some issues, with the gray button lagging behind the rest when it's expanded. If this is fixed it might be ok to add it.

Thanks for the reminder, there is now a bug where the sidebar button is seen as part of the sidebar, causing it to be displayed at the end (lag) during the expanded animation. It should be pinned (split) out. I'll fix it tomorrow :)

@Wulian233
Copy link
Author

Wulian233 commented Jan 21, 2025

Hi Hugo @hugovk , could you review this simple change, the effect of preview website is https://python-docs-theme-previews--210.org.readthedocs.build/en/210/ thanks!

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks, working nicely on macOS with Chrome, Safari, Firefox and Opera, and mobile layout still working on Samsung S22 (I think) with Chrome.

@hugovk
Copy link
Member

hugovk commented Jan 21, 2025

Would be good if someone can give a quick check on Windows, Linux and iPhone.

@Wulian233
Copy link
Author

Record_2025-01-21-19-49-36_a252b927494330cdc2c8ba3b3f952e5e.mp4

It worked fine on my Android phone because sphinx doesn't have a sidebar button and comes with its own animation, so can only see smooth scrolling

@AA-Turner
Copy link
Member

Tested on iPhone. The scroll behaviour when clicking anchor links within a page seems fine, but it is a bit annoying to have a 'scroll' when loading a new page -- e.g. open this link in a new page: https://python-docs-theme-previews--210.org.readthedocs.build/en/210/glossary.html#term-docstring

A

@Wulian233
Copy link
Author

Wulian233 commented Jan 22, 2025

This is good advice, I tried it a few times and now the problem is finally solved. Clicking on an anchor point in the sidebar will smooth scrolling, and directly accessing a link with an anchor point will be pinned directly and will not scroll

https://python-docs-theme-previews--210.org.readthedocs.build/en/210/glossary.html#term-docstring
https://python-docs-theme-previews--210.org.readthedocs.build/en/210/whatsnew/3.14.html#pep-761-discontinuation-of-pgp-signatures

@encukou
Copy link
Member

encukou commented Jan 22, 2025

Happy to see animations added if they're an improvement for most users, but for some, they are harmful.
If you add the animations, could you turn them off for @media (prefers-reduced-motion)? Thanks.

@Wulian233
Copy link
Author

Wulian233 commented Jan 22, 2025

I've never thought about accessibility before, and I read the passage you gave me. I just now commit and theoretically these users will now disable the animation effect

edit: finished

@hugovk
Copy link
Member

hugovk commented Jan 24, 2025

Hmm, it is a little odd that when clicking to open the menu again, at first it shows the menu text before moving the divider back to the right. But maybe it doesn't matter?

image image image image

@Wulian233
Copy link
Author

Very carefully obviously. This animation only lasts for 0.3 seconds, and I didn't notice it 😂. I need to take a closer look at whether the text in the sidebar appears or disappears suddenly, without following the sidebar's animation. I'll check it out tomorrow and hope I can handle this little problem

Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

The reason the sidebar disappears and reappears jarringly is that Sphinx's classic theme sets display: none in sidebar.js. This is used to key the open/close behaviour. I have tried several things to remedy this but short of reimplementing that entire file there aren't great options. This is because the JS in the classic theme has changed over the versions of Sphinx supported by this theme.

As this is a purely cosmetic change, and I don't think we should override the entire sidebar just for animations, I suggest we should probably close this for the time being, and re-visit it if/when we support a narrower range of Sphinx versions, or if we re-implement the sidebar entirely ourselves.

Sorry for wasting your time here @Wulian233, and thank you for being so conscientious.

A

python_docs_theme/static/pydoctheme.css Outdated Show resolved Hide resolved
@Wulian233
Copy link
Author

Wulian233 commented Jan 25, 2025

Never mind! I'm also very willing to continue contributing to other aspects of this project. I just removed the fadeIn animation. Now there are only the imperfect sidebar animation and the color gradient of the hover sidebar button left. Feel free close/merge this pr at any time

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.

5 participants