fix(ExpandableSection): made animation opt-in for detached variant #11851
Conversation
|
Preview: https://patternfly-react-pr-11851.surge.sh A11y report: https://patternfly-react-pr-11851-a11y.surge.sh |
|
First commit shows the original logic where a new Second commit is more true opt-in where isDetached doesn't do anything new unless hasDetachedAnimations is also passed in. Note that I didn't update the direction prop and also didnt remove the pf-m-detached class from the Toggle component file; both of those I can update if this is the route we wanna go with for now |
| @@ -63,7 +67,7 @@ export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionT | |||
| <span | |||
| className={css( | |||
| styles.expandableSectionToggleIcon, | |||
| isExpanded && direction === 'up' && styles.modifiers.expandTop | |||
| isExpanded && direction === 'up' && styles.modifiers.expandTop // TODO: next breaking change move this class to the outter styles.expandableSection wrapper | |||
There was a problem hiding this comment.
Outer misspelled, and is there an issue open for this?
| @@ -32,6 +32,8 @@ import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle | |||
|
|
|||
| When passing the `isDetached` property into `<ExpandableSection>`, you must also manually pass in the same `toggleId` and `contentId` properties to both `<ExpandableSection>` and `<ExpandableSectionToggle>`. This will link the content to the toggle via ARIA attributes. | |||
|
|
|||
| By default animations will not be enabled for a detached `<ExpandableSection>`. You must manually pass the `direction` property with an appropriate value based on where the expandable content is rendered. If the expandable content is above the expandable toggle, `direction="up"` must be passed like in this example. | |||
There was a problem hiding this comment.
I don't mind the hasDetachedAnimations approach if we want to be more true to opt-in, but my main concern is that with how core is currently set up animations will be on by default and be potentially the incorrect direction for detached sections until updated (because pf-m-detached is what is disabling animations from detached and that won't be present until the flag is enabled).
I wonder if it would be better to make animations as a whole opt-in for expandable section with a new modifier instead, and as part of that also require the direction to be passed in for detached?
There was a problem hiding this comment.
cc @mcoker wdyt?
What: Closes #11850
Additional issues: