Skip to content

Improve semantic naming and practices for scrollable code blocks#348

Open
ruslanpashkov wants to merge 1 commit intoexpressive-code:mainfrom
ruslanpashkov:scrollable-module-improvements
Open

Improve semantic naming and practices for scrollable code blocks#348
ruslanpashkov wants to merge 1 commit intoexpressive-code:mainfrom
ruslanpashkov:scrollable-module-improvements

Conversation

@ruslanpashkov
Copy link
Contributor

@ruslanpashkov ruslanpashkov commented Jul 11, 2025

Background

Previous changes addressed Arc Toolkit compliance but introduced axe-core violations with "best practices" rules. After discussion with @delucis (original conversation), we identified that role="region" was inappropriate for scrollable code blocks and needed a more semantic approach.

Changes Made

1. Semantic Module Renaming

  • Renamed: tabindex-js-module.ts → a11y-js-module.ts
  • Reason: Module now handles comprehensive accessibility (tabindex, ARIA attributes, screen reader labels), not just tabindex management

2. Improved Role Semantics

3. Enhanced Screen Reader Support

  • Added: Dynamic <span class="sr-only"> labels with unique IDs
  • Benefits over aria-label:
    • Browser auto-translation support (aria-label doesn't translate properly)
    • Centralized logic in single module (easier maintenance)
    • Uses content-based hashing for stable IDs
    • Proper cleanup when scrollability changes
<!-- Generated structure -->
<pre tabindex="0" role="group" aria-labelledby="ec-label-abc123">
  <code>...</code>
  <span id="ec-label-abc123" class="sr-only">Horizontally scrollable code</span>
</pre>

4. Improved Variable Naming

  • hasTabIndex → isFocusable
  • needsTabIndex → isScrollable
  • updateTabIndex → updateAccessibility

Testing Completed

Screen Readers: VoiceOver, NVDA
Auto-translation: Browser translation works with sr-only labels
Automated Tools: axe-core, Arc Toolkit, WAVE (all passing)
WCAG Compliance: 4.1.2 Name, Role, Value satisfied

Visual Testing Results

Example of generated HTML structure

Generated HTML showing pre element with role='group', tabindex='0', aria-labelledby attribute, and sr-only span with unique ID

VoiceOver now can read the scrollable code blocks easly

VoiceOver reading 'Horizontally scrollable code, group' demonstrating screen reader can access the scrollable code block

Scrollable code blocks now under the Form Controls in VoiceOver Rotor

VoiceOver Rotor showing code blocks listed under Form Controls, proving they are keyboard accessible without cluttering landmarks

No more noise of code blocks in landmark elements

VoiceOver Rotor landmarks list showing only semantic page landmarks (banner, navigation, main, complementary) without code block noise

@netlify
Copy link

netlify bot commented Jul 11, 2025 •
edited
Loading

✅ Deploy Preview for expressive-code ready!

Name Link
🔨 Latest commit bc962da
🔍 Latest deploy log https://app.netlify.com/projects/expressive-code/deploys/68715e86dc2787000893abdf
😎 Deploy Preview https://deploy-preview-348--expressive-code.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ruslanpashkov
Copy link
Contributor Author

ruslanpashkov commented Jul 11, 2025

Initially, it was decided to implement aria-label approach, but during the implementation I realized that too many third-party modules were affected in the process, and it was necessary to add postprocessRenderedBlock for proper localization management. This seemed awkward for simply displaying text to screen readers, so I decided to choose a more practical approach.

@hippotastic
Copy link
Collaborator

hippotastic commented Jul 12, 2025

@delucis Would it be possible for you to review this PR as it was created based on a discussion you had?

@hippotastic hippotastic requested a review from delucis July 12, 2025 08:45
@hippotastic
Copy link
Collaborator

hippotastic commented Aug 23, 2025

Hello @ruslanpashkov, thank you for this PR! Sorry for the long delay in reviewing it.

I've now had a look at it, read through your linked discussion with @delucis, and the changes in this PR look good to me overall. There are two aspects I'd like to discuss:

  • I'm not sure if dynamically adding and removing DOM elements for aria-labelledby is really better than using aria-label. My gut feeling is that dynamically adding an aria-label property through the script when needed would be less intrusive (no dynamic DOM manipulation on resize, less noise in the DOM) and could still be performed by the script (centralized logic). Instead of relying on the browser auto-translation, I'd favor making the generic label localizable through EC so that users can specify alternate labels per code block locale, similar to what plugin-frames is already doing for the copy to clipboard button.

  • When reading "horizontally scrollable code", I wondered if we should just make it "scrollable code" and extend the checks to also include vertically overflowing content, which could happen if authors put a max-height on the code blocks in case of very long code examples. If vertically scrollable blocks also require the same a11y markup as horizontally scrollable ones, I think a generic "scrollable code" handling and naming would be even better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delucis delucis Awaiting requested review from delucis

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ruslanpashkov @hippotastic