-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(datatable): remove toolbar search and menu from tab order #14397
fix(datatable): remove toolbar search and menu from tab order #14397
Conversation
…tch actions are open
@@ -257,6 +256,7 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search( | |||
type={type} | |||
value={value} | |||
tabIndex={onExpand && !isExpanded ? -1 : undefined} | |||
{...rest} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we had this placed before all the existing props. Props passed by consumers would always be overwritten by what we had here.
It's possible this could cause some unintended effects downstream, but I don't think it's necessarily a breaking change. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the only "unintended effect" that props users were passing and expecting to work will now actually work? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is breaking either, more like a bug fix for something that should have been working.
✅ Deploy Preview for carbon-components-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 ✅
@tay1orjones just needs updated snaps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This removes the table toolbar search and menu actions from the tab order when batch actions are open, as discussed in #14357 (comment)
Changelog
Changed
...rest
after other propsTesting / Reviewing
datatable.focus.batch.actions.mov