fix(theme-common): handle disabled collapsible transitions#12053
fix(theme-common): handle disabled collapsible transitions#12053rohan-patnaik wants to merge 1 commit into
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
slorber
left a comment
slorber
left a comment
There was a problem hiding this comment.
This overly complexify the code for what we attempt to do.
I'd rather find a simpler solution, one that doesn't add complexity and one that doesn't require reading computed styles.
This may mean we offer a more convenient way to achieve the initial use-case
See also my comment here: #12043 (comment)
| el.style.height = collapsedStyles.height; | ||
| } | ||
|
|
||
| function parseTransitionTime(time: string) { |
| const onCollapseTransitionEndRef = useRef(onCollapseTransitionEnd); | ||
|
|
||
| useEffect(() => { | ||
| onCollapseTransitionEndRef.current = onCollapseTransitionEnd; | ||
| }, [onCollapseTransitionEnd]); |
| function finishAnimation() { | ||
| applyCollapsedStyle(el, collapsed); | ||
| onCollapseTransitionEndRef.current?.(collapsed); | ||
| } | ||
|
|
||
| function finishAnimationIfTransitionDisabled() { | ||
| const {transitionDuration} = getComputedStyle(el); | ||
| if (parseTransitionTime(transitionDuration) === 0) { | ||
| finishAnimation(); | ||
| } | ||
| } |
|
Thanks for the review. I agree the current approach adds more complexity than this use case probably deserves, especially around reading computed styles. I’ll hold off on adding more code until the preferred direction is clear. I see two simpler options:
Do you prefer one of those directions, or is there another simpler approach you’d rather see? |
2 participants
Fixes #12043
Summary
This makes the shared
Collapsiblecomponent finish its expand/collapse cleanup when the computed transition duration is0ms.Why
Before this change,
Collapsiblewaited for aheighttransition event before applying its final styles. If site CSS forcedtransition: 0ms !important, that event did not fire, so expanded sidebar categories could keep temporary height/overflow styles and visually overlap later items.Test plan
nvm use 26 && yarn oxfmt --list-different packages/docusaurus-theme-common/src/components/Collapsible/index.tsxnvm use 26 && yarn eslint --cache --report-unused-disable-directives packages/docusaurus-theme-common/src/components/Collapsible/index.tsxgit diff --check