-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[RFR] Remove BulkActions in V3 #3517
Conversation
Kunnu01
commented
Aug 13, 2019
•
edited
Loading
edited
- Remove BulkActions
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.
Mind you add a word in UPGRADE.md about this component?
There is a dedicated section about removed deprecated components that were removed
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.
Removing this component also implies updating the List
component which used it
|
Yes
Yes :) |
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.
Almost there! 👍
@@ -52,7 +51,6 @@ const ListToolbar = ({ | |||
React.cloneElement(actions, { | |||
...rest, | |||
className: styles.actions, | |||
bulkActions, |
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.
I think you'll have to update the default ListActions too
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.
Good! I'll wait for @fzaninotto review though, just to be sure :)
Neat! I think the Upgrade guide should be a bit more explicit, by showing a code snippet with a before/after diff. |
Okay, I'll update it. Do you want it in a specific format or should I just simply add code snippets saying |
Check the rest of the UPGRADE file: we use snippets with the |
Done. Please review. :) |
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! |