Skip to content

fix(accessibility): group search filters into single section landmark and remove individual facet landmarks (#5497) - #6172

Open
MoamenElbarqy wants to merge 2 commits into
DSpace:mainfrom
MoamenElbarqy:fix/5497-search-filters-landmarks
Open

fix(accessibility): group search filters into single section landmark and remove individual facet landmarks (#5497)#6172
MoamenElbarqy wants to merge 2 commits into
DSpace:mainfrom
MoamenElbarqy:fix/5497-search-filters-landmarks

Conversation

@MoamenElbarqy

Copy link
Copy Markdown

References

Description

This PR resolves accessibility landmark overload in the search filters by removing role="region" from individual facet filters and grouping the entire filter component under a single labeled <section> landmark.

Instructions for Reviewers

List of changes in this PR:

  • Removed role="region" from SearchFilterComponent (search-filter.component.html) so individual facets (e.g. Author, Subject, Date, Item Type) are no longer exposed as separate region landmarks.
  • Wrapped SearchFiltersComponent (search-filters.component.html) in a semantic <section [attr.aria-label]="filterLabel + '.filters.head' | translate"> landmark to provide a single labeled region for the entire search filters panel.
  • Added accessibility unit tests in search-filter.component.spec.ts and search-filters.component.spec.ts.

How to test:

  1. Run the search filters unit tests:
    npm test -- --include="src/app/shared/search/search-filters/**/*.spec.ts"

Copilot AI lite review requested due to automatic review settings September 1, 2026 23:32
@MoamenElbarqy

Copy link
Copy Markdown
Author

Apologies for the delay on this! The PR is ready for review, looking forward to your feedback.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes align with the stated accessibility goal and the added unit tests directly validate the new landmark behavior.

Pull request overview

This PR addresses accessibility “landmark overload” in the search filter sidebar by removing per-facet region landmarks and introducing a single, labeled <section> landmark that groups the entire filters panel.

Changes:

  • Wrapped SearchFiltersComponent template content in a <section> with a translated aria-label to provide a single landmark for the filter panel.
  • Removed role="region" from the per-facet filter container in SearchFilterComponent to avoid exposing each facet as a separate landmark.
  • Added unit tests asserting the presence of the section landmark label and the absence of role="region" on individual facets.
File summaries
File Description
src/app/shared/search/search-filters/search-filters.component.html Introduces a single labeled <section> landmark wrapping the filter panel.
src/app/shared/search/search-filters/search-filters.component.spec.ts Adds an accessibility unit test for the section landmark and non-empty aria-label.
src/app/shared/search/search-filters/search-filter/search-filter.component.html Removes role="region" from individual facet containers to reduce landmark noise.
src/app/shared/search/search-filters/search-filter/search-filter.component.spec.ts Adds a unit test asserting facets are not exposed as region landmarks.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lgeggleston lgeggleston added bug accessibility component: Discovery related to discovery search or browse system 1 APPROVAL pull request only requires a single approval to merge port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release port to dspace-10_x This PR needs to be ported to `dspace-10_x` branch for next bug-fix release labels Sep 2, 2026
@lgeggleston lgeggleston moved this to 🙋 Needs Reviewers Assigned in DSpace 11.0 Release Sep 2, 2026
@lgeggleston

Copy link
Copy Markdown
Contributor

Hi @MoamenElbarqy, thank you for your contribution to DSpace! This is added to a project board for reviewers to take a look.

One request, could you please add some instructions to manually test this fix, in this case how to view the difference before and after of the region landmarks structure of the page? I'm also seeing that the issue author didn't specify which tool they used to see the landmark issue, but as long as it's reproducing and fixing the same issue any tool you used to test would work. Thank you!

@MoamenElbarqy

Copy link
Copy Markdown
Author

Hi @lgeggleston

Here are the step-by-step instructions to manually test and inspect the landmark structure before and after this fix:

Recommended Tool:

You can use the Landmark Navigation via Keyboard or Pop-up Chrome extension.


Step-by-Step Testing Guide:

  1. Navigate to the Search Page:

    • Open http://localhost:4000/search (or any search page with facet filters).
  2. Open Landmark Navigation:

    • Using the Landmark Navigation extension: Click the extension icon in the toolbar (or press Alt + Shift + N) to open the list of landmarks.

Expected Behavior (Before vs After):

  • Before (on main / demo.dspace.org):

    • Each individual filter facet (e.g., Author, Subject, Date issued, Has files, Item Type) is announced as a separate region landmark, causing landmark overload.
  • After (with this PR):

    • Individual facet region landmarks are removed.
    • The entire search filter panel is neatly grouped under a single Filters (or Search filters) region landmark.

Here is a screenshot demonstrating the resolved landmark structure:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge accessibility bug component: Discovery related to discovery search or browse system port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release port to dspace-10_x This PR needs to be ported to `dspace-10_x` branch for next bug-fix release

Projects

Status: 🙋 Needs Reviewers Assigned

Development

Successfully merging this pull request may close these issues.

[Accessibility]: Filter controls are exposed as separate landmark regions, causing landmark overload

3 participants