fix(ExpandableSection): made animation opt-in for detached variant#7550
fix(ExpandableSection): made animation opt-in for detached variant#7550mcoker merged 3 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-7550.surge.sh A11y report: https://patternfly-pr-7550-a11y.surge.sh |
|
Can also test how it works in React by using this draft PR patternfly/patternfly-react#11851 |
24dc37b to
db9cd23
Compare
srambach
left a comment
There was a problem hiding this comment.
Just minor nits in docs. Looks good to me otterwise. 🦦
| transform: rotate(var(--#{$expandable-section}__toggle-icon--Rotate)); | ||
|
|
||
| &.pf-m-expand-top { | ||
| &.pf-m-expand-top { // TODO: Remove this block in breaking change in favor of using modifier on outter expandable section wrapper |
There was a problem hiding this comment.
| &.pf-m-expand-top { // TODO: Remove this block in breaking change in favor of using modifier on outter expandable section wrapper | |
| &.pf-m-expand-top { // TODO: Remove this block in breaking change in favor of using modifier on outer expandable section wrapper |
😛 🦦
There was a problem hiding this comment.
You and Katie calling me outt 😭 (that misspelling was intentional at least)
It's funny because I think I told Katie the same thing about maybe having otter's on the mind 😆
| --#{$expandable-section}__toggle-icon--Rotate: var(--#{$expandable-section}__toggle-icon--m-expand-top--Rotate); | ||
| } | ||
|
|
||
| &:has(.#{$expandable-section}__content:only-child) { |
There was a problem hiding this comment.
Can you add a comment here about the assumption we're making about the structure with the only-child?
| | `.pf-m-indented` | `.pf-v6-c-expandable-section` | Indicates that the expandable section content is indented and is aligned with the start of the title text to provide visual hierarchy. | | ||
| | `.pf-m-truncate` | `.pf-v6-c-expandable-section` | Indicates that the expandable section content is truncated by default, and not truncated when expanded. | | ||
| | `.pf-m-expand-top` | `.pf-v6-c-expandable-section__toggle-icon` | Modifies the toggle icon to point up when expanded. | | ||
| | `.pf-m-expand-top` | `.pf-v6-c-expandable-section__toggle-icon` | Modifies the toggle icon to point up when expanded. We recommend the new method of applying this class directly to the `.pf-v6-c-expandable-section` wrapper element. | |
There was a problem hiding this comment.
wdyt about moving this adjacent to the other .pf-m-expand-top so that a reader would see them together? I just wonder if it could be confusing if one is missed.
db9cd23 to
0ace978
Compare
|
🎉 This PR is included in version 6.3.0-prerelease.33 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #7515
Some notes:
In React I believe we're fine to add things outside of breaking changes -- wasn't sure if we wanted to apply thePer discussion in retroplanning 6/4/2025, React won't have the detached class as it won't be needed for animations. Right now it's kept in Core though not marked as required; the plan is in the next breaking change to have those classes applied+required for detached variantspf-m-detachedmodifier to thepf-v6-c-expandable-sectionwrapper that only contains the toggle, but I did that here which should be fine to add into React + sorta seemed odd that only one of the wrapper would have that modifier