Skip to content
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

Update focus order for search result page #209

Merged
merged 8 commits into from
Apr 23, 2024

Conversation

mikejritter
Copy link
Contributor

What does this do?

  • Render the sidebar toggle button as part of the sidebar
  • Update css to keep sidebar styling consistent with previous behavior

Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1309

This issue came about from the wcag audit with respect to the logical ordering of elements when tabbing through the page. According to the audit the expected behavior is for the tab input to go through the search result table first, then navigate to the sidebar. The toggle bar for the sidebar was created before the sidebar which caused it to be tabbed to before the search result table, which is out of order according to the audit.

How should this be tested? Do these changes have associated tests?

  • Run the devserver
  • Test the behavior for a simple search
    • Navigate to the search page and execute a search
    • Tab through the page and see that the order goes from the navbar, to the search results, to the sidebar toggle + sidebar
    • Toggle and untoggle the sidebar and see that the behavior is consistent with dev
  • Test the behavior for and advanced search
    • Navigate to the search page and execute an advanced search
    • Tab through the page and see that the order goes from the navbar, to the search results, to the sidebar toggle + sidebar
    • Toggle and untoggle the sidebar and see that the behavior is consistent with dev

Dependencies for merging? Releasing to production?
These changes also need to be made for the record page before we can close the Jira, as the issue exists on both pages. I'll add a comment on the ticket as well so that it's notated there.

In addition I believe these changes will be applicable to DRYD-1295 as well because it updates the DOM to be ordered correctly. I still need to download NVDA and test screen reading.

Has the application documentation been updated for these changes?
No

Did someone actually run this code to verify it works?
@mikejritter tested locally

Copy link

codecov bot commented Apr 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.20%. Comparing base (dd6fee1) to head (dd1b282).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #209   +/-   ##
=======================================
  Coverage   98.20%   98.20%           
=======================================
  Files         555      555           
  Lines       12504    12505    +1     
  Branches     2586     2583    -3     
=======================================
+ Hits        12280    12281    +1     
  Misses        221      221           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@ray-lee ray-lee left a comment

Choose a reason for hiding this comment

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

Looks good!

@ray-lee ray-lee merged commit 47f500b into collectionspace:master Apr 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants