fixes header action selector #523
Conversation
|
👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack. |
|
@kvendrik is this still a change you'd like to see merged? I'd recommend rebasing and re-pinging for review if so. Otherwise it should be closed. |
kvendrik
requested review from
solonaarmstrong-zz
and removed request for
solonaarmstrong-zz
| onClose={this.handleActionGroupClose} | ||
| active={title === openActionGroup} | ||
| /> | ||
| <div className={styles.IndividualAction} key={title}> |
There was a problem hiding this comment.
Should this have a more unique key similar to the one given to secondary actions on line 270?
| <div className={styles.IndividualAction} key={title}> | |
| <div | |
| className={styles.IndividualAction} | |
| key={`ActionGroup-${title}-${index}`} | |
| > |
| details={details} | ||
| onOpen={this.handleActionGroupOpen} | ||
| onClose={this.handleActionGroupClose} | ||
| active={title === openActionGroup} |
There was a problem hiding this comment.
Should this logic be in a variable?
There was a problem hiding this comment.
I would say yes although I think in this specific case its okay to leave it as is considering storing this in a variable would involve changing this into a block which would arguably make it harder to read.
5 participants
WHY are these changes introduced?
fixes https://github.com/Shopify/polaris-react-deprecated/issues/2463.
moved from https://github.com/Shopify/polaris-react-deprecated/pull/2487.
WHAT is this pull request doing?
Replaces the wildcard action selector with an actual selector that points to secondary actions in the page header.
How to 🎩
actionGroupsandsecondaryActionsto the Playground.🎩 checklist