Refactor Quick Actions for tables #18535
Conversation
| <ProjectActionBox isPublished={rowData.isPublished}/> | ||
| <QuickAction | ||
| text={i18n.rename()} | ||
| action={()=>{}} |
There was a problem hiding this comment.
This table isn't done and is currently only in storybook, adding placeholder actions for now.
There was a problem hiding this comment.
this is fine for a placeholder. once you have actual functions in here, you'll want to define them outside the render method so that we don't re-bind to this on every render
caleybrock
force-pushed
the
quick-actions-box
branch
from
dbe2d7a
to
4f73fcc
Compare
caleybrock
force-pushed
the
quick-actions-box
branch
from
df2c396
to
c2f9018
Compare
| text={i18n.remix()} | ||
| action={()=>{}} | ||
| /> | ||
| {this.props.isPublished && |
There was a problem hiding this comment.
These lines when the published flag determines what children to show seems to cause an error, but I've vet to figure out why.
Error: Call to console.error from "Already published project": Warning: Failed prop type: Invalid prop `children[3]` of type `boolean` supplied to `QuickActionsBox`, expected a single ReactElement.
There was a problem hiding this comment.
Oh I see, you can't have a function as a child. Will update.
|
@davidsbailey Fixed that test so this should be ready for review now, thanks! |
|
👀 looking... |
| <ProjectActionBox isPublished={rowData.isPublished}/> | ||
| <QuickAction | ||
| text={i18n.rename()} | ||
| action={()=>{}} |
There was a problem hiding this comment.
this is fine for a placeholder. once you have actual functions in here, you'll want to define them outside the render method so that we don't re-bind to this on every render
| @@ -8,7 +8,7 @@ import wrappedSortable from '../tables/wrapped_sortable'; | |||
| import orderBy from 'lodash/orderBy'; | |||
| import {PROJECT_TYPE_MAP, personalProjectDataPropType} from './projectConstants'; | |||
| import QuickActionsCell from '../tables/QuickActionsCell'; | |||
| import ProjectActionBox from './ProjectActionBox'; | |||
| import QuickAction from '../tables/QuickAction'; | |||
There was a problem hiding this comment.
after removing ProjectActionBox from this file, it looks like it is only used by ProjectCard.jsx (and it's own storybook entry). This seems fine, just confirming my understanding.
There was a problem hiding this comment.
Yes, that's correct. Mostly just didn't want to remove any storybook functionality from the project card, and this is at least as good as it was before.
Refactored the quick action cell components I made for the
PersonalProjectsTable(still only in storybook) a while back to be something that can be used in any table, with any set of actions. I also added it to theManageStudentsTable, which is in active development.I know @epeach is currently working on adding some actions to the SectionsTable, and while she copied the styles from the old version of this component, she could not use it because it was not general enough. Hopefully this makes things a little cleaner for both of us.
Erin, I'm happy to pair with you when you're back to get our code down to one version of this component. I think our main difference is the implementation of how the box actually pops up and gets dismissed, so we should probably take a look at both and see which one feels smoother. For now though, this unblocks me.
Stand alone video:
Screenshots (both only in storybook):
ManageStudentsTable
PersonalProjectsTable