[Collapsible] Add Collapsible components and hooks#481
Merged
colmtuite merged 12 commits intomui:masterfrom Aug 28, 2024
Merged
[Collapsible] Add Collapsible components and hooks#481colmtuite merged 12 commits intomui:masterfrom
colmtuite merged 12 commits intomui:masterfrom
Conversation
Netlify deploy preview |
69f291b to
397ff68
Compare
397ff68 to
30e8cc0
Compare
e78e435 to
3638e68
Compare
atomiks
reviewed
Aug 6, 2024
packages/mui-base/src/Collapsible/Content/useCollapsibleContent.ts
Outdated
Show resolved
Hide resolved
0172ef8 to
939767f
Compare
939767f to
8296f06
Compare
8296f06 to
244d479
Compare
a6676a5 to
e83e63a
Compare
michaldudak
reviewed
Aug 21, 2024
| return renderElement(); | ||
| }); | ||
|
|
||
| export namespace CollapsibleContent { |
| setHeight(rect.height); | ||
| } | ||
|
|
||
| element.style.animationName = shouldCancelAnimation ? 'none' : originalAnimationName; |
Member
There was a problem hiding this comment.
I'm wondering if we need to do all this on our side. Have you considered applying the data-instant attribute and instructing users to disable all motion when it's present? (perhaps this could also solve the issue with transitions)
Member
Author
There was a problem hiding this comment.
@michaldudak data-instant applies conditions that last for the lifetime of the component, which is different for the cases that are handled internally here, which only disables motion for one specific instance/occurrence of the open state changing:
- When the component is initially open, on first mount/load, motion is disabled so it's just appears fully open, but animations run normally after that
- When the component uses
hidden="until-found"and is opened by in-page-search, it will open instantly, but animations run normally afterwards
I don't think these behaviors should require users to opt-in to it (or be override-able) for animated Collapsibles as it obviously results in poorer UX
Also this keeps the component easier to style, without having to add another animation state in addition to the 4 standard ones!
(I've also added a comment in the code about this here)
949ca4f to
c5289c7
Compare
c5289c7 to
3e0f351
Compare
3e0f351 to
696867a
Compare
1 task
michaldudak
approved these changes
Aug 26, 2024
Member
Author
|
@colmtuite It's ready for you to have one last look before we merge it ~ |
colmtuite
approved these changes
Aug 28, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #84
Docs: https://deploy-preview-481--base-ui.netlify.app/base-ui/react-collapsible/
Extra demos:
Collapsibles + CSS transitions: https://deploy-preview-481--base-ui.netlify.app/experiments/collapsible-accordion/hidden="until-found": https://deploy-preview-481--base-ui.netlify.app/experiments/collapsible-hidden-until-found/