Add aria-label to directory search/facets if label is hidden#324
Add aria-label to directory search/facets if label is hidden#324markconroy wants to merge 4 commits into2.xfrom
Conversation
|
@msayoung wants to review this |
There was a problem hiding this comment.
Pull Request Overview
Adds accessibility improvements by ensuring hidden labels on directory search and facets blocks are exposed via aria-label, and assigns appropriate roles.
- Introduces a preprocess function to detect hidden labels on specific blocks
- Sets
aria-labelandroleattributes based on block configuration
Comments suppressed due to low confidence (2)
localgov_microsites_base.theme:693
- The docblock should include
@param array $variablesand referencehook_preprocess_blockto conform with Drupal coding standards.
/**
localgov_microsites_base.theme:696
- Consider adding a render or functional test to verify that
aria-labelandroleare correctly output whenlabel_displayis hidden.
function localgov_microsites_base_preprocess_block(&$variables) {
| if ($variables['base_plugin_id'] === 'localgov_directories_channel_search_block' || 'facet_block') { | ||
| if ($variables['elements']['#configuration']['label_display'] === '0') { | ||
| $variables['attributes']['aria-label'] = $variables['elements']['#configuration']['label']; | ||
| $variables['attributes']['role'] = 'search'; |
There was a problem hiding this comment.
[nitpick] Applying role='search' to the facets block can be misleading. Scope the role assignment to only the search block or choose a more appropriate ARIA role for facets (e.g., region).
| $variables['attributes']['role'] = 'search'; | |
| $variables['attributes']['role'] = 'region'; |
|
Very cool @markconroy We're using the block title as the aria-label, if the block title isn't visible. I know that's the block title, and it can be changed, but because its not visible people are unlikely to change it. I don't have an idea of what would be better though. cc @waako @Mayur2609 (as you both were involved in the original ticket). |
|
@waako @Mayur2609 any opinions on this one? |
|
Agreed with @msayoung that using the Block title is probably too generic unfortunately, as it won't mean much to users. Maybe we can get the Page title (h1), and suffix that to the block type ? so end up with aria-label something like:
CoPilot is correct that the |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@msayoung @waako have we got an agreement on what we want to update this to? Also, I think role=search on the block is the correct approach. Adrian Roselli agrees - https://adrianroselli.com/2015/08/where-to-put-your-search-role.html
|
|
I'm happy with @waako 's suggestion for the aria label. "Search {{node:title}}" makes sense. Great article from Adrian, lets go with what he advises on the search role. |
|
@msayoung This is ready for another test now. |
|
Just thinking about this - surely we want this in LocalGov Base rather than LocalGov Microsites Base? |
|
Here's the same PR for LocalGov Base: localgovdrupal/localgov_base#843 I know the <div
class="facet-inactive contextual-region block-facet--checkbox block block-facets block-facet-blocklocalgov-directories-facets"
id="block-localgov-directories-facets-scarfolk--2"
aria-label="Search National Parks in the UK"
role="search">
...
</div> |
|
Closing this for now in favour of localgovdrupal/localgov_base#843 |
Closes #322
What does this change?
Checks if the directory channel search block and/or directory channel facets block have their label hidden. If so, add the label as an
aria-labelso screen reader users will understand what these blocks are, since they come before the H1 in the page.