Refactor polaris-button-group component #447
Conversation
| @action | ||
| setupAutoWrapper(element) { | ||
| // TODO When we're auto-wrapping we bypass the group item component, so we will not handle | ||
| // focus events and hence not apply correct class to the wrapper group item |
There was a problem hiding this comment.
Is this a new issue? When does it occur/what are the symptoms?
Also thinking... can we ever handle this without some (probably hacky) workaround? 🤔
There was a problem hiding this comment.
This is an existing issue....whenever users of this component are not explicitly wrapping buttons inside buttonGroup.item components, we will auto-wrap buttons with a div (what we do here), but that obviously will not handle focus events that the buttonGroup.item component does
Can't think of something nice here, only if we could easily render the item component inline and wrap with it contents....sounds crazy
I'd say it might worth just deprecating this auto-wrapping for components like this (not sure if there are other)
There was a problem hiding this comment.
Yeah, probably removing auto-wrapping is the way to go to work around this, any other "fix" would probably have to be dirty 😬
| segmented=(not persistActions) | ||
| data-test-id=(unless persistActions "shortcut-actions") | ||
| <PolarisButtonGroup | ||
| data-test-id={{unless persistActions "shortcut-actions"}} |
There was a problem hiding this comment.
| data-test-id={{unless persistActions "shortcut-actions"}} | |
| data-test-id={{unless @persistActions "shortcut-actions"}} |
?
There was a problem hiding this comment.
we will get to this component too shortly....this was just to get tests passing
No description provided.