Skip to content

Siteimprove a11y fixes - #117

Open
Dananji wants to merge 3 commits into
mainfrom
siteimprove-a11y-fixes
Open

Siteimprove a11y fixes#117
Dananji wants to merge 3 commits into
mainfrom
siteimprove-a11y-fixes

Conversation

@Dananji

@Dananji Dananji commented Jul 24, 2026

Copy link
Copy Markdown

Related issue: #84
Related a11y issue in #83

Changes in this PR:

  • add accessible name to ColourSwatchPicker button via aria-label used by both the annotation edit form and the settings modal
  • add a label prop to TimePicker and apply aria-label to each of its hour/minute/second <input> fields
  • replace useVolumeSlider with a new generalized useSliderA11y module, with a function to patch MUI's Slider a11y in class components (bubble height slider in SettingsPopup) and a custom hook to apply the patch to functional components (VolumeSlider and VolumeSliderCompact)
  • cleanup the onChange and onToggle events that were in useVolumeSlider and move them inside each relevant component. The MUI's event handlers are partially working for the keydown events, therefore handle the mouse/touch event handler portion inside the callback functions in respective component
  • add a new Inert component that sets native inert DOM attribute on a wrapped sub-tree of components when a modal is open. This makes all interactive elements in the whole sub-tree unfocusable and unclickable, which clears the SiteImprove flags on the background elements being active when a modal is open. AudioImporter and SettingsPopup modal elements are rendered as siblings of <Inert>
  • cleanup isModalOpen prop in VariationsAppBar, AudioTransportBar, BubbleEditor, TimelineScrubber, Metadata, and ProjectMetadataDisplay components where, it is only used for disabling buttons/links when the modal is open. Removing isModalOpen in these places is safe because, Inert component blocks focus/click actions on the interactive elements in these components. Other props and variables used in these disabled attribute in these components are left untouched because they don't rely on the modal visibility.

@cjcolvar cjcolvar 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.

This looks great as far as I can tell by just looking at the code. Nice job refactoring to clean things up and deal with issues in MUI 3!

Comment on lines +17 to +18
/* React 16 doesn't recognize 'inert' as a valid HTML attribute, therefore to safely
implement 'inert' use a React ref on the HTML element. */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tricky!

*/
export const handleFocusTrap = (event, isOpen) => {
if (event.key !== 'Tab' || !open) return;
if (event.key !== 'Tab' || !isOpen) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good catch!

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