fix(settings): make codec priority reorderable by keyboard and touch - #743
Open
czoli1976 wants to merge 3 commits into
Open
fix(settings): make codec priority reorderable by keyboard and touch#743czoli1976 wants to merge 3 commits into
czoli1976 wants to merge 3 commits into
Conversation
The codec priority list was HTML5 drag-and-drop only, with no keyboard handlers, ARIA roles or touch support. Keyboard users could not reorder the codecs at all (WCAG 2.1.1), and because HTML5 drag events never fire on touch, the control was unusable on phones and tablets - where this dialog renders full screen. Add per-item Move up / Move down buttons with descriptive aria-labels, disabled at the ends of the list, reusing the existing pure reorder helper. Drag and drop is untouched for mouse users. Also add an idPrefix prop so the field can be rendered twice with unique test ids, which the upcoming screen-share codec section needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AI Adoption Report
Per-commit breakdown (3 commits)
Warning The following commits have no Git AI authorship notes — AI adoption cannot be tracked for these changes.
Powered by Git AI Standard v3.0.0 — authorship data from |
The Move up / Move down buttons used the `disabled` attribute at the ends of the list. Moving a codec to either end disables the very button that was just activated, and the browser then drops focus from a disabled element to <body> - verified in Chrome 148: focusing a button and setting `disabled` leaves document.activeElement as document.body, while `aria-disabled` retains it. So the final press of a keyboard-only reorder left the user with no focus at all, and the next Tab restarted from the top of the dialog. That is a focus order defect introduced by the very change meant to make this control keyboard accessible. Mark the boundary buttons `aria-disabled` instead. They stay focusable and in the tab order while still announcing themselves as unavailable, and moveCodec already no-ops at the bounds so pressing them does nothing. The disabled-state styling moves from the `disabled:` variants to a conditional class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These three keys - advancedSettings.video.sections.camera.label and the two screenSharing ones - were added here by accident. Nothing in the codec priority field reads them; they belong with the change that introduces the Camera and Screen Sharing section headings, and have been moved there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What is this PR doing?
Makes the codec priority list in Settings → Video reorderable by keyboard and on touch.
The list was HTML5 drag-and-drop only — no key handlers, no ARIA, no touch path:
Two consequences:
handler anywhere in the field — a WCAG 2.1.1 Keyboard (Level A) failure.
phones and tablets — where this dialog renders full-screen — the setting can be read but never
changed.
This adds a Move up / Move down button to each row, disabled at the ends of the list, each
with a translated
aria-labelnaming the codec it acts on ("Move VP9 up"). Both delegate to theexisting pure
reorderCodecPriorityhelper, so ordering behaviour is unchanged and there is nosecond implementation to keep in sync:
Drag-and-drop is deliberately untouched, so mouse users lose nothing.
One piece of scope that is not accessibility. The field's
data-testids were hardcoded, so thecomponent could only ever be rendered once per page. This adds an
idPrefixprop (defaulting totoday's value, so nothing changes) to make a second instance addressable. That is groundwork for a
screen-share codec section; it is included here because it touches the same lines as the testids
above and would otherwise conflict.
New keys land in all five locale files in this commit, so
localeParity.spec.tsstays green.How should this be manually tested?
codec should change position and the numbering update. On
developthere is nothing to Tab to.developdragging doesnothing at all.
Verified manually in the browser across all four paths.
Known and deliberately out of scope
Flagged so they read as decisions rather than oversights:
nothing is announced to a screen reader (WCAG 4.1.3 Status Messages, AA). Worth adding an
aria-liveregion, but it is a separate concern from the Level A keyboard failure this fixes.44px of 2.5.5 (AAA). Sized to match the existing row furniture rather than introduce a
one-off.
What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDSOL-
Checklist
[x] Branch is based on
develop(notmain).[ ] Resolves a
Known Issue.[ ] If yes, did you remove the item from the
docs/KNOWN_ISSUES.md?[x] Resolves an item reported in
Issues.If yes, which issue? #739