[terra-button-group] Updated tabIndex on button group for single select example #4022
Conversation
|
Keyboard navigation for single select buttons should be similar to radio button fields ie: Tab should put focus on the first button and subsequent tabbbing should move the focus out of the group to the next focusable element. Only arrow keys should work inside the group for single select buttons.Removing the entire tabIndex logic means that functioanlity is breaking. |
Updated. |
|
Focus should come onto the selected item (when present) initially but as per these changes, it will always come onto the first button. |
Updated |
| @@ -170,7 +170,10 @@ class ButtonGroup extends React.Component { | |||
|
|
|||
| React.Children.forEach(children, (child, index) => { | |||
| const isSelected = selectedKeys.indexOf(child.key) > -1; | |||
| const tabAttr = (this.props.onChange && !isMultiSelect) ? { tabIndex: (isSelected) ? '0' : '-1' } : null; | |||
| // Checking whehter any option is selected or not | |||
There was a problem hiding this comment.
nit:
| // Checking whehter any option is selected or not | |
| // Checking whether any option is selected or not |
| @@ -170,7 +170,10 @@ class ButtonGroup extends React.Component { | |||
|
|
|||
| React.Children.forEach(children, (child, index) => { | |||
| const isSelected = selectedKeys.indexOf(child.key) > -1; | |||
| const tabAttr = (this.props.onChange && !isMultiSelect) ? { tabIndex: (isSelected) ? '0' : '-1' } : null; | |||
| // Checking whehter any option is selected or not | |||
| const isAnyOptionSelected = children && Array.isArray(children) ? children.findIndex((e) => this.props.selectedKeys.includes(e?.key)) : -1; | |||
There was a problem hiding this comment.
| const isAnyOptionSelected = children && Array.isArray(children) ? children.findIndex((e) => this.props.selectedKeys.includes(e?.key)) : -1; | |
| const isAnyOptionSelected = children && Array.isArray(children) ? children.findIndex((child) => this.props.selectedKeys.includes(child?.key)) : -1; |
|
Please add a wdio test to verify that focus moves to the first button when no buttons are selected initially in a single select button group. |
Added. |
| @@ -170,7 +170,11 @@ class ButtonGroup extends React.Component { | |||
|
|
|||
| React.Children.forEach(children, (child, index) => { | |||
| const isSelected = selectedKeys.indexOf(child.key) > -1; | |||
| const tabAttr = (this.props.onChange && !isMultiSelect) ? { tabIndex: (isSelected) ? '0' : '-1' } : null; | |||
| // Checking whether any option is selected or not | |||
| // eslint-disable-next-line no-shadow | |||
There was a problem hiding this comment.
Why is this eslint rule disabled?
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| * Removed | |||
| * Removed `tabAttr` variable to fix tabbing issue on the button group. | |||
There was a problem hiding this comment.
| * Removed `tabAttr` variable to fix tabbing issue on the button group. | |
| * Fixed | |
| * Fixed tabbing issue for button-group without initial focus. |
| @@ -3,6 +3,7 @@ | |||
| ## Unreleased | |||
|
|
|||
| * Added | |||
| * Added a test example to validate focus when none of the buttons are selected. | |||
There was a problem hiding this comment.
| * Added a test example to validate focus when none of the buttons are selected. | |
| * Added a test example for `terra-button-group` to validate focus when none of the buttons are selected. |
|
+1 |
Summary
Previously, we are unable to tab (using tab key) between buttons on the button group in single select example. With this fix it has been resolved and we can move focus to the button even it is not selected on single select example.
What was changed:
Updated tabIndex on button group for single select example.
Why it was changed:
User unable to tab between buttons on button group.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-10171
Thank you for contributing to Terra.
@cerner/terra