fix(search): switch search role from datatable to search, add labeledby #5330
Conversation
ghost
requested review from
|
Deploy preview for carbon-elements ready! Built with commit 241dcc9 |
|
Deploy preview for carbon-components-react ready! Built with commit 241dcc9 https://deploy-preview-5330--carbon-components-react.netlify.com |
| @@ -162,9 +162,12 @@ export default class Search extends Component { | |||
| const CloseIconX = size === 'xl' ? Close20 : Close16; | |||
|
|
|||
| return ( | |||
| <div className={searchClasses}> | |||
| <div | |||
| role="search" | |||
There was a problem hiding this comment.
Should this always be on the <Search> component? I'm just not sure what the impact is 🤔
It seems like role="search" is treated as a landmark and if folks are using this frequently it might clutter the list of regions on the page if we default here versus over in TableToolbarSearch we should have ideally one version of itself on thepage.
There was a problem hiding this comment.
From what I read, it seems like even if there are multiple searches on a page, each one still should be a landmark with a unique label of what it is for (data table, the whole site, etc). Since we didn't make the input type "search", the Search component isn't a landmark right now and probably should be without users needing to add it.
| <Search16 className={`${prefix}--search-magnifier`} /> | ||
| <label htmlFor={id} className={`${prefix}--label`}> | ||
| <label id={id + '-search'} htmlFor={id} className={`${prefix}--label`}> |
There was a problem hiding this comment.
Maybe it'd help out to create a variable like const searchId = ${id}-search; and use it in both spots? Just so updates to one location are mirrored in the other location.
There was a problem hiding this comment.
Good idea! I'll add it!
Closes #5289
Closes: #4665
This PR removes
role="search"from TableToolbarSearch and adds it to the outer div of the search component and adds anidto the label so that the search is labeled bylabelTextand can be made unique to each Search bar if there are multiple on a page, for example if someone has a search component for the page and one inside a data table.Changelog
New
idfor SearchlabelChanged
Removed
Testing / Reviewing
Inspect the Search component in or outside of a data table and check that
aria-labelledbywith a unique id is being applied to the div withrole="search".