-
Notifications
You must be signed in to change notification settings - Fork 28
Enhancements for dropdowns: animations, open at initial state, and chevron indicators #675
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
base: main
Are you sure you want to change the base?
Enhancements for dropdowns: animations, open at initial state, and chevron indicators #675
Conversation
✅ Deploy Preview for scientific-python-hugo-theme ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
8802455
to
f15829b
Compare
Thanks for working on this @agriyakhetarpal! I think the addition of the chevron indicators would help users in recognising that the dropdowns are expandable 😄 I'm wondering if we can change the speed of the chevron animation on hover? From what I can see, the chevron has a fade-in animation, but the background colour change seems to be quicker. I think it would be nice if both animations ended at the same time. I think this is also the case in the PST dropdown animations. This is a small nitpick though, overall I think the improvements are great. Thanks again! |
I appreciate the feedback, @smeragoel! You are right, there is a slight delay between the darkening of the dropdown bar background on hover and the scaling of the chevrons kicking in. However, this is because the PST implementation in https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/web-components.html#dropdowns does not include animations; the dropdown bar's background changes colour and the chevrons do not animate when I've gone ahead and implemented your suggestion with the above commit to match PST – the chevrons should no longer scale when hovering and thus end at the same time as the dropdown's background colour change. Could you PTAL when it is convenient for you? To be completely honest: I did notice this difference when working on the changes here, and I've quite come to like the slight deviation that I've made from PST – I feel that it's nice that the chevrons here also animate on hovering rather than abruptly scaling in size without an animation. However, I understand that I would be biased here as I'm familiar with my PR more :) Could you educate me about any accessibility concerns that may arise with the previous case (i.e., before this commit) as I may not be aware of them? Thank you! |
Looks good to me, thanks @agriyakhetarpal! Could we make the chevron animation go in the other direction? It feels to me like, if the content scrolls down the chevron should rotate clockwise. This is already true for the sideways chevron, but not the up-down one. Just my aesthetic feeling :) |
Yes, certainly, I agree. This is also the case with the GitHub PR checks we can see above the comment box, they rotate clockwise in a 180-degree rotation and I like them too :) |
Actually, wait, the I ask this because I have no strong opinion here; the last example in https://sphinx-design.readthedocs.io/en/pydata-theme/dropdowns.html#more-examples is the down-up case, and it does not have an animation at all. I chose to deviate from it and add an animation as I felt that it was better to have one. :) |
The GitHub directions feel intuitive to me, so I'd mimic that. |
One thing we should also address here before merging: the FontAwesome icons can take their due time to load on throttled networks based on the limited simulations I performed and on what I noticed with several hard refreshes (without caches) – the dropdown bars will show up first and the chevrons way later on a slow network. Maybe it makes sense to vendor these specific icons? I think PST is doing this, too. |
Yes, either that, or you may be able to use unicode chevrons. They should look good enough. |
I addressed the animation changes in the commit above; it should now resemble the GitHub interface. I tried the Unicode chevron https://www.compart.com/en/unicode/U+276F, which looks like this: ❯ However, it's not as nice (and not the same as the FontAwesome ones either), and no available rotated equivalent retains the same shape. Vendoring should be the preferred approach here, for parity with PST. Edit: argh, okay, so it looks like PST is using webpack to do this: https://github.com/pydata/pydata-sphinx-theme/blob/eef26c8c4c80b23e846dbcd2ebc9804093341f8c/webpack.config.js. We could vendor only the specific chevron icons for their inline SVGs, but I don't want to leave things in an inconsistent state where we vendor two symbols and not the rest of them. I think it makes sense for us to go down the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All done (for now). This is ready for another look! Here is a self-review for some explanations.
-moz-user-select: none; | ||
-ms-user-select: none; | ||
-webkit-user-select: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My text editor warns that these attributes are non-standard and should be avoided, and user-select
is baseline already. The same warnings occur in a few other places below, and I removed the browser-engine-specific styling there as well.
&:not([open]) > .sd-summary-title:hover .sd-summary-chevron-right i { | ||
transform: scale(1.1); | ||
} | ||
|
||
&:not([open]) > .sd-summary-title:hover .sd-summary-chevron-down i { | ||
transform: scale(1.1); | ||
} | ||
|
||
&[open] > .sd-summary-title:hover .sd-summary-chevron-right i { | ||
transform: rotate(90deg) scale(1.1); | ||
} | ||
|
||
&[open] > .sd-summary-title:hover .sd-summary-chevron-down i { | ||
transform: rotate(-180deg) scale(1.1); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the earlier commit I pushed today, I changed this to .sd-summary-title:hover
as we want to increase the chevron sizes only when hovering over the dropdown bars and not when hovering over the content inside them (this is the same as PST).
In particular, these lines address the cases where you hover over the chevrons and click on them – we don't want the chevrons to transition from a 1.1x scale to a 1.0x scale while rotating and then back to a 1.1x scale, but rather stay at a 1.1x scale throughout the transition.
-moz-animation: sd-fade-in 0.5s ease-in-out; | ||
-webkit-animation: sd-fade-in 0.5s ease-in-out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above – these are non-standard attributes and are best avoided. I can also clean these up from everywhere in a separate PR and reduce the diff here if you want me to.
<span class="sd-summary-state-marker {{ $chevronClass }}"> | ||
<i class="fas {{ $chevronIcon }}"></i> | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.sd-summary-state-marker
is a separate element that makes it easier for us to style where the chevrons are placed (at the end of the dropdown bars).
Description
Tip
Try it out at https://deploy-preview-675--scientific-python-hugo-theme.netlify.app/shortcodes/#dropdown
This PR modifies the
dropdown
shortcode to add three new features, in order to establish feature parity with the dropdowns from the PyData Sphinx Theme and the sphinx-design projects. This was also discussed in #477 (comment), and should resolve that part of the conversation. Also, this PR is an aftermath of #471.In particular, the features added here are as follows:
In addition to this, I've added several more examples to the shortcode's docs: a success dropdown, a warning one, a danger one, a multi-line example, one with code, and lastly, a nested dropdown.
I've checked for any accessibility issues/implications, and I think this should be fine; I've closely followed the PST/SD implementation(s), and the chevrons here work similarly.