Remove unnecessary aria-label defaults from Details component #3534

Copilot · 2025-06-10T20:14:01Z

Problem

The Details component was always setting aria-label attributes with default values ("Expand"/"Collapse"), even when users didn't explicitly provide them. This created accessibility issues:

Screen readers already communicate the expanded/collapsed state of <details> elements
The aria-label was overriding the visible text content, potentially causing conflicts
Users had no way to opt out of these labels to let screen readers use the natural element behavior

Solution

Modified the Details component to only set aria-label attributes when explicitly provided by users:

Ruby Component Changes

Only sets data-aria-label-* attributes when aria_label_open or aria_label_closed are provided
Only sets initial aria-label when explicit aria-labels are given
Preserves aria-expanded behavior for proper state communication

TypeScript Element Changes

Only sets aria-label during toggle when data-aria-label-* attributes exist
Removes fallback to default values ("Expand"/"Collapse")
Updated documentation to reflect new behavior

Before (problematic):

<summary aria-label="Expand" data-aria-label-closed="Expand" data-aria-label-open="Collapse">
  Click me
</summary>

After (improved):

<!-- No aria-labels when not explicitly provided -->
<summary aria-expanded="false">
  Click me
</summary>

<!-- Still works when explicitly provided -->
<summary aria-label="Open details" data-aria-label-closed="Open details" data-aria-label-open="Close details">
  Click me  
</summary>

Testing

Updated existing tests to expect no default aria-labels
Added tests to verify explicit aria-labels still work correctly
Added tests to verify aria-labels are absent when not provided
Confirmed Dropdown component (which uses Details internally) continues working correctly
All component tests pass: Details (17 tests), Dropdown (8 tests)

Benefits

Screen readers can use visible text content instead of redundant aria-labels
Better accessibility compliance with WCAG guidelines
Users can opt out of aria-labels entirely when appropriate
Maintains full backward compatibility when aria-labels are explicitly provided

Fixes #3533.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

accounts.google.com
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4729 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-nm8t1u --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,7696498983390953603,2146872180022467109,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
- Triggering command: /opt/google/chrome/chrome --headless --disable-gpu --hide-scrollbars --mute-audio --enable-automation --disable-web-security --disable-session-crashed-bubble --disable-breakpad --disable-sync --no-first-run --use-mock-keychain --keep-alive-for-test --disable-popup-blocking --disable-extensions --disable-component-extensions-with-REDACTED-pages --disable-hang-monitor --disable-features=site-per-process,IsolateOrigins,TranslateUI --disable-translate --disable-REDACTED-networking --enable-features=NetworkService,NetworkServiceInProcess --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-client-side-phishing-detection --disable-default-apps --disable-dev-shm-usage --disable-ipc-flooding-protection --disable-prompt-on-repost --disable-renderer-REDACTEDing --disable-site-isolation-trials --force-color-profile=srgb --metrics-recording-only --safebrowsing-disable-auto-update --password-store=basic --no-startup-window --remote-debugging-port=0 --remote-debugging-address=127.0.0.1 --window-size=1024,768 --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-nm8t1u --noerrdialogs --ozone-platform=headless --ozone-override-screen-size=800,600 --use-angle=swiftshader-webgl (dns block)
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4759 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-57js7g --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,15882939688443903777,17075563584011957770,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
clients2.google.com
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4729 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-nm8t1u --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,7696498983390953603,2146872180022467109,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4759 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-57js7g --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,15882939688443903777,17075563584011957770,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
- Triggering command: /opt/google/chrome/chrome --headless --disable-gpu --hide-scrollbars --mute-audio --enable-automation --disable-web-security --disable-session-crashed-bubble --disable-breakpad --disable-sync --no-first-run --use-mock-keychain --keep-alive-for-test --disable-popup-blocking --disable-extensions --disable-component-extensions-with-REDACTED-pages --disable-hang-monitor --disable-features=site-per-process,IsolateOrigins,TranslateUI --disable-translate --disable-REDACTED-networking --enable-features=NetworkService,NetworkServiceInProcess --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-client-side-phishing-detection --disable-default-apps --disable-dev-shm-usage --disable-ipc-flooding-protection --disable-prompt-on-repost --disable-renderer-REDACTEDing --disable-site-isolation-trials --force-color-profile=srgb --metrics-recording-only --safebrowsing-disable-auto-update --password-store=basic --no-startup-window --remote-debugging-port=0 --remote-debugging-address=127.0.0.1 --window-size=1024,768 --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-57js7g --noerrdialogs --ozone-platform=headless --ozone-override-screen-size=800,600 --use-angle=swiftshader-webgl (dns block)
safebrowsingohttpgateway.googleapis.com
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4729 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-nm8t1u --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,7696498983390953603,2146872180022467109,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
- Triggering command: /opt/google/chrome/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=4759 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-57js7g --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,15882939688443903777,17075563584011957770,262144 --enable-features=NetworkService,NetworkServiceInProcess --disable-features=IsolateOrigins,PaintHolding,TranslateUI,site-per-process --variations-seed-version (dns block)
- Triggering command: /opt/google/chrome/chrome --headless --disable-gpu --hide-scrollbars --mute-audio --enable-automation --disable-web-security --disable-session-crashed-bubble --disable-breakpad --disable-sync --no-first-run --use-mock-keychain --keep-alive-for-test --disable-popup-blocking --disable-extensions --disable-component-extensions-with-REDACTED-pages --disable-hang-monitor --disable-features=site-per-process,IsolateOrigins,TranslateUI --disable-translate --disable-REDACTED-networking --enable-features=NetworkService,NetworkServiceInProcess --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-client-side-phishing-detection --disable-default-apps --disable-dev-shm-usage --disable-ipc-flooding-protection --disable-prompt-on-repost --disable-renderer-REDACTEDing --disable-site-isolation-trials --force-color-profile=srgb --metrics-recording-only --safebrowsing-disable-auto-update --password-store=basic --no-startup-window --remote-debugging-port=0 --remote-debugging-address=127.0.0.1 --window-size=1024,768 --user-data-dir=/tmp/ferrum_user_data_dir_20250611-4700-57js7g --noerrdialogs --ozone-platform=headless --ozone-override-screen-size=800,600 --use-angle=swiftshader-webgl (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

changeset-bot · 2025-06-10T20:14:05Z

🦋 Changeset detected

Latest commit: e0e0e33

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Co-authored-by: jonrohan <[email protected]>

jonrohan · 2025-06-10T20:42:20Z

@copilot address the test failures in test/system/beta/details_test.rb

Co-authored-by: jonrohan <[email protected]>

Copilot · 2025-06-10T20:51:01Z

@copilot address the test failures in test/system/beta/details_test.rb

Fixed the system test failures in test/system/beta/details_test.rb. The tests were expecting default aria-labels that no longer exist after the accessibility improvements. Updated the tests to:

Verify no aria-labels are present by default (as intended by the fix)
Test that explicit aria-labels still work correctly when provided
Added a new preview for testing explicit aria-label behavior

The tests now correctly validate the new behavior where aria-labels are only set when explicitly provided by users. Commit: b4dc8ee

Copilot

Pull Request Overview

This PR refactors the Details component to stop injecting default aria-label attributes unless users explicitly provide them, improving accessibility by allowing screen readers to rely on native element behavior.

Ruby component now only adds aria-label and data-aria-label-* when explicit values are passed
TypeScript toggle logic updated to conditionally set aria-label based on data-aria-label-* presence
System and component tests updated to verify absence of default labels and correct explicit behavior
Preview updated to demonstrate explicit aria-label usage

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/system/beta/details_test.rb	Updated system tests to assert no default `aria-label` and added checks for explicit labels
test/components/beta/details_test.rb	Added component tests for partial, absent, and explicit `aria-label` scenarios
previews/primer/beta/details_preview.rb	Introduced preview variant demonstrating explicit aria-label usage
app/components/primer/beta/details_toggle_element.ts	Changed toggle logic to only set `aria-label` when `data-aria-label-*` attrs exist
app/components/primer/beta/details.rb	Modified component to only include `aria-label` and `data-aria-label-*` when provided
.changeset/popular-lizards-attack.md	Added patch summary for removing default `aria-label` behavior

Comments suppressed due to low confidence (2)

app/components/primer/beta/details.rb:32

[nitpick] Add or update doc comments for the Details component to explain that aria_label_closed and aria_label_open are now optional and only applied when explicitly provided, and that default labels are no longer set.

class Details < Primer::Component

test/system/beta/details_test.rb:28

[nitpick] Rename this test method to mirror the click test naming (e.g., test_no_aria_labels_by_default_on_keyboard) for consistency and clarity.

def test_no_aria_labels_by_default_keyboard

Copilot · 2025-06-10T21:06:32Z

app/components/primer/beta/details_toggle_element.ts

-      this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)
+      const ariaLabelClosed = this.summaryTarget.getAttribute('data-aria-label-closed')
+      if (ariaLabelClosed) {
+        this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)


Ensure that when no data-aria-label-closed is provided, any existing aria-label attribute is removed to avoid stale labels. For example, call this.summaryTarget.removeAttribute('aria-label') in the branch where ariaLabelClosed is falsy.

Suggested change

this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)

this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)

} else {

this.summaryTarget.removeAttribute('aria-label')

doesn't seem like this would hurt but not sure if it can get in this state 🤔

rsese

thanks for jumping on this one so quick ✨ !

I scanned through the changes and it makes sense to me, and will make it super easy to opt-out of aria-labels. Will leave a 👍 since also dropped extensive tests for the behavior but np if you want approval from other folks ✌️

rsese · 2025-06-10T22:07:47Z

test/components/beta/details_test.rb

+    # Should not have aria-label attribute
+    refute_selector("summary[aria-label]")
+    # Should not have data-aria-label attributes 
+    refute_selector("summary[data-aria-label-closed]")
+    refute_selector("summary[data-aria-label-open]")
+    # Should still have aria-expanded
+    assert_selector("summary[aria-expanded=false]")


is pretty verbose with the comments 😅

rsese · 2025-06-10T22:34:13Z

app/components/primer/beta/details.rb

+        # Only add aria-label if explicitly provided
+        if aria_label_closed || aria_label_open
+          current_label = open? ? (aria_label_open || ARIA_LABEL_OPEN_DEFAULT) : (aria_label_closed || ARIA_LABEL_CLOSED_DEFAULT)
+          aria_attributes[:label] = current_label
+        end


ahh ok I was little confused at first but the test clarified -- so if you provide only one label but not the other, the state without a label will use a default label 👍

rsese · 2025-06-10T22:55:32Z

app/components/primer/beta/details_toggle_element.ts

-      this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)
+      const ariaLabelClosed = this.summaryTarget.getAttribute('data-aria-label-closed')
+      if (ariaLabelClosed) {
+        this.summaryTarget.setAttribute('aria-label', ariaLabelClosed)


doesn't seem like this would hurt but not sure if it can get in this state 🤔

Initial plan for issue

324f3f3

Copilot AI assigned Copilot and jonrohan Jun 10, 2025

Copilot started work on behalf of jonrohan June 10, 2025 20:14 View session

Remove unnecessary aria-label defaults from Details component

b7ed975

Co-authored-by: jonrohan <[email protected]>

Copilot AI changed the title ~~[WIP] Details component has unnecessary aria-label~~ Remove unnecessary aria-label defaults from Details component Jun 10, 2025

Copilot AI requested a review from jonrohan June 10, 2025 20:27

Copilot finished work on behalf of jonrohan June 10, 2025 20:27

Create popular-lizards-attack.md

2bfc123

github-actions bot added the patch release label Jun 10, 2025

Copilot started work on behalf of jonrohan June 10, 2025 20:42 View session

Fix system tests to expect no default aria-labels

b4dc8ee

Co-authored-by: jonrohan <[email protected]>

Copilot finished work on behalf of jonrohan June 10, 2025 20:52

jonrohan requested review from rsese and khiga8 June 10, 2025 21:04

jonrohan marked this pull request as ready for review June 10, 2025 21:04

Copilot AI review requested due to automatic review settings June 10, 2025 21:04

jonrohan requested a review from a team as a code owner June 10, 2025 21:04

Copilot AI reviewed Jun 10, 2025

View reviewed changes

Merge branch 'main' into copilot/fix-3533

e0e0e33

rsese approved these changes Jun 10, 2025

View reviewed changes

jonrohan enabled auto-merge June 10, 2025 23:12

francinelucca approved these changes Jun 10, 2025

View reviewed changes

jonrohan added this pull request to the merge queue Jun 10, 2025

Merged via the queue into main with commit b8ebe04 Jun 10, 2025
27 checks passed

jonrohan deleted the copilot/fix-3533 branch June 10, 2025 23:26

jonrohan temporarily deployed to preview June 10, 2025 23:26 — with GitHub Actions Inactive

primer bot mentioned this pull request Jun 10, 2025

Release Tracking #3519

Merged

openprojectci mentioned this pull request Jun 17, 2025

Release Tracking opf/primer_view_components#324

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove unnecessary aria-label defaults from Details component #3534

Remove unnecessary aria-label defaults from Details component #3534

Uh oh!

Copilot AI commented Jun 10, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Jun 10, 2025 •

edited

Loading

Uh oh!

jonrohan commented Jun 10, 2025

Uh oh!

Copilot AI commented Jun 10, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 10, 2025

Uh oh!

rsese Jun 10, 2025

Uh oh!

rsese left a comment

Uh oh!

rsese Jun 10, 2025

Uh oh!

rsese Jun 10, 2025

Uh oh!

rsese Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

Remove unnecessary aria-label defaults from Details component #3534

Remove unnecessary aria-label defaults from Details component #3534

Uh oh!

Conversation

Copilot AI commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Ruby Component Changes

TypeScript Element Changes

Before (problematic):

After (improved):

Testing

Benefits

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

changeset-bot bot commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

jonrohan commented Jun 10, 2025

Uh oh!

Copilot AI commented Jun 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

rsese Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

rsese left a comment

Choose a reason for hiding this comment

Uh oh!

rsese Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

rsese Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

rsese Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jun 10, 2025 •

edited

Loading

changeset-bot bot commented Jun 10, 2025 •

edited

Loading