Skip to content

a11y: expose Satus settings controls semantically - #4332

Open
imparable2022 wants to merge 1 commit into
code-charity:masterfrom
imparable2022:a11y/menu-semantics
Open

imparable2022 wants to merge 1 commit into
code-charity:masterfrom
imparable2022:a11y/menu-semantics

Conversation

@imparable2022

Copy link
Copy Markdown
Contributor

Summary

Improve keyboard and screen-reader semantics in the ImprovedTube settings UI without changing existing settings behavior or storage.

This adds an accessibility compatibility layer for the Satus-rendered menu and loads it from the popup and side-panel UI.

What changes

  • expose custom switches with role="switch", keyboard activation and aria-checked
  • give native selects, checkboxes, radios, sliders and text/time controls accessible names from their Satus skeleton labels
  • expose shortcut/color-picker controls as keyboard-operable buttons
  • label icon-only buttons and hide decorative SVGs from the accessibility tree
  • expose titled sections as labelled regions with headings
  • add accessible iframe titles
  • add visible focus indicators for keyboard navigation
  • add an initial document language to the menu HTML
  • dynamically enhance newly inserted controls with MutationObserver

Scope

This PR intentionally does not change layer-entry focus, dialog behavior, YouTube-page injected controls, locale selection, translations, or feature behavior. Those are split into follow-up PRs so each decision can be reviewed independently.

Validation

The complete accessibility series was manually tested in Chromium with NVDA using the unpacked extension. Existing Satus rendering and storage keys are preserved.

@wahajahmed010

Copy link
Copy Markdown

Strong scope discipline (no behavior changes, accessibility is additive) and the scope-of-future-PRs callout is helpful for reviewers. Two notes on the implementation:

  1. Auto-install side effect on import. The IIFE at the top of menu/accessibility.js calls api.install() whenever the module loads in a non-CommonJS environment with a global document:
if (root && root.document && typeof module === 'undefined') api.install();

That means any test, dev snippet, or future popup that imports this file for any reason — even just to inspect ImprovedTubeMenuAccessibility — will start a MutationObserver and start mutating the DOM. The recommended pattern is to export install/uninstall and only run install from the explicit load sites (popup, side-panel). Tests can still call install directly via the global. The current shape makes the module un-importable for inspection, which is a real cost.

  1. MutationObserver lifetime is unclear from the visible diff. The PR description says "dynamically enhance newly inserted controls with MutationObserver" but I do not see a matching observer.disconnect() in the visible portion. If the observer lives for the lifetime of the popup/side-panel page, that's fine, but the install/uninstall pair should keep them paired so a re-install (e.g. after a settings update) doesn't accumulate observers and double-apply aria-* attributes. The same install/uninstall separation above also fixes this.

The semantic-enhancement functions themselves are solid — accessibleName/setLabel precedence is reasonable, role="switch" plus aria-checked is the correct pattern, and the icon-name fallback via extension.skeleton is a thoughtful way to label existing Satus buttons without re-rendering them.

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