-
Notifications
You must be signed in to change notification settings - Fork 20
feat/841: add optional codemod and flag for enabling animations #843
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
Conversation
Signed-off-by: gitdallas <[email protected]>
In: | ||
|
||
```jsx | ||
import { AlertGroup, TreeView } from '@patternfly/react-core'; | ||
import { Table } from '@patternfly/react-table'; | ||
|
||
export const Example = () => ( | ||
<> | ||
<AlertGroup isLiveRegion> | ||
{/* alerts */} | ||
</AlertGroup> | ||
<TreeView /> | ||
<Table /> | ||
</> | ||
); | ||
``` | ||
|
||
Out: | ||
|
||
```jsx | ||
import { AlertGroup, TreeView } from '@patternfly/react-core'; | ||
import { Table } from '@patternfly/react-table'; | ||
|
||
export const Example = () => ( | ||
<> | ||
<AlertGroup hasAnimations isLiveRegion> | ||
{/* alerts */} | ||
</AlertGroup> | ||
<TreeView hasAnimations /> | ||
<Table hasAnimations /> | ||
</> | ||
); | ||
``` |
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.
Nit: if you put in
In:
```jsx
%inputExample%
```
Out:
```jsx
%outputExample%
```
my readme builder will insert the content of the *Input and *Output tsx files into the compiled readme
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.
Some of this I'm not sure if we need to link t some release documentation or some of our examples that have been updated to include these animations, but some stuff that we would either need to or would be good to update/expand on:
- AlertGroup: idk if we need the callout here or maybe in release highlights (I can mention this in Erin's PR for that too), but enabling animations has a chance to break tests depending if consumers have any/how they're setup. This is due to how using animations tweaks when/how some callbacks are actually called.
- DualListSelector animations are only applicable when
isTree
prop is true, otherwise thehasAnimations
prop shouldn't be passed in. - Table: there might be more of a lfit from consumers to have the "correct" structure for animations to work correctly. We can still add the hasAnimations prop fine, but depending how they're structuring their table they may have to look at our examples (and their changes since enabling animations) to get them working as expected. PR 11865 shows the changes just we had to do in our examples for animations
Also maybe worth updating the rule README to mention that some components' DOM structure will change when animations are enabled.
closes #841