-
Notifications
You must be signed in to change notification settings - Fork 20
fixes to hasAnimations codemod #845
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
fixes to hasAnimations codemod #845
Conversation
Signed-off-by: gitdallas <[email protected]>
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.
Thanks for making these changes @gitdallas - left some comments below, otherwise LGTM.
<Table /> | ||
</> | ||
); | ||
%inputExample% |
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.
Just a quick question on this syntax - is this expected? Or would this need to be updated to include examples of the components updated in this PR?
%inputExample% | |
%inputExample% |
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.
It is, I've got a script that compiles all of these separate readme's together for the "real" readme and inserts the associated tsx files into these spots
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.
Super cool - thank you for the context here @wise-king-sullyman
<Table hasAnimations /> | ||
</> | ||
); | ||
%outputExample% |
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.
Same here as well
@@ -42,11 +42,56 @@ module.exports = { | |||
const message = | |||
"Consider adding hasAnimations prop to enable component animations."; | |||
|
|||
// Helper function to check if isTree prop exists and isn't explicitly false | |||
function hasValidIsTreeProp(node: JSXOpeningElement): boolean { |
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.
Not that it would be a blocker for me, but there are some helpers in /helpers/JSXAttributes.ts
that would probably let you not have as much custom logic in this rule for evaluating the isTree
prop if you weren't aware of them.
Signed-off-by: gitdallas <[email protected]>
fixes some notes from post-merge reviews on #843 as well as an issue that came up where
hasAnimations
is supposed to go on SearchInputExpandable instead of SearchInput.