Project Table : add quick actions box #16994
Conversation
|
cc @Erin007 who originally worked on this. |
|
@Bjvanminnen it's possible you missed a notification for this one. |
|
I did miss this somehow. My first reaction is I wonder whether this component should be the same as #17007 (or if it makes sense to have two different dropdown types components). |
| this.setState({ | ||
| actionsOpen: false | ||
| }); | ||
| }.bind(this)); |
There was a problem hiding this comment.
could use an arrow function to avoid needing to bind
There was a problem hiding this comment.
I do kind of like this .one approach.
| <div style={styles.cellContainer}> | ||
| <div | ||
| style={selectedStyle} | ||
| ref={(icon) => { this.icon = icon; }} |
There was a problem hiding this comment.
nit: for one liners like this, you don't actually need the braces, i.e.
ref={icon => this.icon = icon}
| let actions = [i18n.rename(), i18n.remix()]; | ||
| this.props.isPublished ? | ||
| actions.push(i18n.removeFromPublicGallery()) : actions.push(i18n.publishToPublicGallery()); | ||
| return actions; |
There was a problem hiding this comment.
I think this would be more clearly expressed as:
return [
i18n.rename(),
i81n.remix(),
this.props.isPublished ? i18n.removeFromPublicGallery() : i18n.publishToPublicGallery()
];
| {this.state.actionsOpen && | ||
| <ProjectActionBox | ||
| isPublished={this.props.projectData.publishedToPublic} | ||
| style={{position: 'absolute', marginLeft: 10, marginTop: '-10px'}} |
There was a problem hiding this comment.
nit: i would put this style in our styles object (in general, i would advocate for doing so any time you have more than one style field)
also, you use a number for marginLeft but a string for marginTop?
|
Updated code review feedback and chatted with Poorva/Mark who box think we should have separate styles for these. |
|
@Bjvanminnen did you want to take another look at this? |
There was an early version of a quick actions box for the project card that is currently unused. I made it it's own component, updated to use the most recent designs and text, and used the new component both for the card and the table.
This affects no user facing features, but is visible in the storybook.
None of these components are linked to actions or redux yet (I'm trying to keep these PRs small because I might be switching to another project before I finish converting this entirely.)