Add keyboard navigation to alphabetical and changes lists - #2023
Conversation
2 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| this.conceptInFocus -= 1 | ||
| } | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } |
| this.conceptInFocus -= 1 | ||
| } | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2023 +/- ##
=========================================
Coverage 70.42% 70.42%
Complexity 1703 1703
=========================================
Files 34 34
Lines 4436 4436
=========================================
Hits 3124 3124
Misses 1312 1312 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds keyboard navigation (Arrow up/down, Home, End, Space) to the sidebar concept lists on the Alphabetical and Changes tabs, partially addressing accessibility issue #1982. Tab enters/exits the list (only the focused item is in the tab order via roving tabindex), and Cypress tests verify the basic up/down behavior on both tabs.
Changes:
- Implement roving-tabindex keyboard navigation in
tab-alpha.jsandtab-changes.jsvia a newconceptInFocusdata property, per-link refs, and ahandleKeydownEventmethod on each list link. - Introduce
indexedConcepts/changedConceptsLengthcomputed properties intab-changes.jsto assign a unique focus index to each link (includingreplacedBytargets). - Add Cypress tests in
sidebar-alpha.cy.jsandsidebar-changes.cy.jsthat exercise Tab + Arrow Down + Arrow Up.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| resource/js/tab-alpha.js | Adds conceptInFocus state, per-link refs and tabindex, and arrow/home/end/space keyboard handler for the alphabetical concept list. |
| resource/js/tab-changes.js | Adds conceptInFocus state, computed indexedConcepts / changedConceptsLength, per-link refs and tabindex, and keyboard handler covering both prefLabel and replacedBy links. |
| tests/cypress/template/sidebar-alpha.cy.js | Extends the alphabetical-index test to assert Tab/ArrowDown/ArrowUp focus behavior on the loaded list. |
| tests/cypress/template/sidebar-changes.cy.js | Adds a new keyboard-navigation test for the changes tab covering Tab/ArrowDown/ArrowUp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| handleKeydownEvent (e) { | ||
| if (e.key === ' ') { | ||
| // Click on link currently in focus | ||
| e.preventDefault() | ||
| this.$refs['concept' + this.conceptInFocus][0].click() | ||
| } else if (e.key === 'ArrowDown') { | ||
| // On last element move focus to first list item, otherwise next list item | ||
| e.preventDefault() | ||
| this.conceptInFocus = (this.conceptInFocus + 1) % this.changedConceptsLength | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } else if (e.key === 'ArrowUp') { | ||
| // On first element move focus to last list item, otherwise to previous list item | ||
| e.preventDefault() | ||
| if (this.conceptInFocus === 0) { | ||
| this.conceptInFocus = this.changedConceptsLength - 1 | ||
| } else { | ||
| this.conceptInFocus -= 1 | ||
| } | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } else if (e.key === 'End') { | ||
| // Move focus to last list item | ||
| e.preventDefault() | ||
| this.conceptInFocus = this.changedConceptsLength - 1 | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } else if (e.key === 'Home') { | ||
| // Move focus to first list item | ||
| e.preventDefault() | ||
| this.conceptInFocus = 0 | ||
| this.$refs['concept' + this.conceptInFocus][0].focus() | ||
| } | ||
| } |
| changedConceptsLength () { | ||
| return [...this.changedConcepts.values()] | ||
| .flat() | ||
| .reduce((acc, entry) => acc + 1 + (entry.replacedBy ? 1 : 0), 0) | ||
| } |
osma
left a comment
osma
left a comment
There was a problem hiding this comment.
I think it looks good and seems to work based on the testing I did.
The Cypress tests should also check that opening concepts with the keyboard is working properly.
I asked Copilot for a review. The second comment it generated is worth checking out - is it possible to simplify the calculations?
| cy.get('.aria-live-message').invoke('text').should('equal', 'Concepts loaded for letter B') | ||
| // Check that new concepts are loaded | ||
| cy.get('#tab-alphabetical').find('.sidebar-list li').first().invoke('text').should('contain', 'birch bark manuscripts') | ||
|
|
There was a problem hiding this comment.
This test only verifies moving between entries, but not opening them (with space and/or enter). I think that's pretty crucial functionality so it should be tested as well.
|
|
||
| }) | ||
| }) | ||
| it('Keyboard navigation', () => { |
There was a problem hiding this comment.
Ditto, test opening (with space/enter) as well.
github-project-automation
Bot
moved this from In progress
to Issue/PR closed
in Skosmos 3.x Backlog
Reasons for creating this PR
Concept lists in sidebar are not currently navigable using the keyboard. This PR adds keyboard navigation to alphabetical and changes tabs.
Link to relevant issue(s), if any
Description of the changes in this PR
Known problems or uncertainties in this PR
Checklist
.sr-onlyclass, color contrast)