fix(global-header): scope accessibility test to #global-header element

[rohitratannagar]

Description

Raised using bug-fix skill

Replaced the blanket disableRules(['nested-interactive']) workaround in the global-header accessibility test with a targeted AxeBuilder.include('#global-header') scope. The 211 nested-interactive violations originate from the upstream Backstage catalog table (react-beautiful-dnd adds role="button" to draggable column headers, which nest inside MUI TableSortLabel buttons), not from the global header itself. By scoping the axe analysis to only the #global-header element, the test now validates the header's own accessibility without suppressing any axe rules.

Root cause

The nested-interactive axe-core violations came from the Backstage catalog table rendered below the global header — react-beautiful-dnd adds role="button" to draggable <th> elements, which already contain MUI <TableSortLabel> buttons. The previous fix disabled the nested-interactive rule entirely, masking any real violations in the global header. The <AppBar id="global-header"> component itself has a clean interactive structure with no nested controls.

Fixed

• RHDHBUGS-2247 — Accessibility Violation in Global Header – Nested Interactive Controls

UI before changes

screenrecordings_global-header-RHDHBUGS-2247_before-fix.1.webm

UI after changes

screenrecordings_global-header-RHDHBUGS-2247_after-fix.1.webm

Checklist

• A changeset describing the change and affected packages. (more info)
• Added or Updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)

Note

This bug fix was identified and implemented using the bug-fix and raise-pr agent skills. Please verify the fix thoroughly before merging.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.35%. Comparing base (70f61da) to head (198cc77).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3949      +/-   ##
==========================================
- Coverage   57.35%   57.35%   -0.01%     
==========================================
  Files        2384     2384              
  Lines       95608    95605       -3     
  Branches    26699    26711      +12     
==========================================
- Hits        54832    54830       -2     
+ Misses      39238    39236       -2     
- Partials     1538     1539       +1     

Flag	Coverage Δ		*Carryforward flag
adoption-insights	84.54% <ø> (ø)		Carriedforward from 70f61da
ai-integrations	69.26% <ø> (ø)		Carriedforward from 70f61da
app-defaults	69.79% <ø> (ø)		Carriedforward from 70f61da
augment	46.67% <ø> (ø)		Carriedforward from 70f61da
boost	75.89% <ø> (ø)		Carriedforward from 70f61da
bulk-import	72.61% <ø> (ø)		Carriedforward from 70f61da
cost-management	13.55% <ø> (ø)		Carriedforward from 70f61da
dcm	60.72% <ø> (ø)		Carriedforward from 70f61da
extensions	56.28% <ø> (ø)		Carriedforward from 70f61da
global-floating-action-button	71.18% <ø> (ø)		Carriedforward from 70f61da
global-header	62.17% <ø> (-0.02%)	⬇️
homepage	47.58% <ø> (ø)		Carriedforward from 70f61da
install-dynamic-plugins	56.77% <ø> (ø)		Carriedforward from 70f61da
intelligent-assistant	74.05% <ø> (ø)		Carriedforward from 70f61da
konflux	91.98% <ø> (ø)		Carriedforward from 70f61da
lightspeed	69.02% <ø> (ø)		Carriedforward from 70f61da
mcp-integrations	83.40% <ø> (ø)		Carriedforward from 70f61da
orchestrator	62.67% <ø> (ø)		Carriedforward from 70f61da
quickstart	65.18% <ø> (ø)		Carriedforward from 70f61da
sandbox	79.56% <ø> (ø)		Carriedforward from 70f61da
scorecard	82.66% <ø> (ø)		Carriedforward from 70f61da
theme	83.85% <ø> (ø)		Carriedforward from 70f61da
translations	5.12% <ø> (ø)		Carriedforward from 70f61da
x2a	79.31% <ø> (ø)		Carriedforward from 70f61da

*This pull request uses carry forward flags. Click here to find out more.

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70f61da...198cc77. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rhdh-qodo-merge]

PR Summary by Qodo

Scope global-header axe scan to #global-header instead of disabling nested-interactive

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Scope axe analysis to the "#global-header" element to avoid unrelated page violations.
• Remove the blanket nested-interactive rule disablement so header regressions are not masked.
• Keep accessibility scan results attached to the Playwright test artifacts as before.

Diagram

graph TD
  T["Playwright E2E test"] --> U["runAccessibilityTests()"] --> B["AxeBuilder (wcag tags)"] --> S(("Include scope: #global-header")) --> P["Page DOM"]
  B --> R(("Axe results JSON"))
  U --> A["TestInfo.attach()"] --> R
  subgraph Legend
    direction LR
    _proc["Test / function"] ~~~ _scope(("DOM scope")) ~~~ _art(("Artifact"))
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep disabling axe nested-interactive rule

• ➕ Fastest workaround; no need to reason about page composition
• ➕ Avoids flakiness from upstream components generating violations
• ➖ Masks real nested-interactive regressions in the global header
• ➖ Weakens the test’s signal and makes future a11y issues harder to catch

2. Exclude the known-problematic catalog table subtree (axe exclude())

• ➕ Still tests most of the page while avoiding the specific offender
• ➕ Avoids blanket rule disablement
• ➖ More brittle: depends on selectors/structure of upstream Backstage content
• ➖ Can expand over time into a long exclusion list

3. Render/test the header in an isolated route/story for a11y scan

• ➕ Most deterministic: test only the component under test
• ➕ Avoids coupling to unrelated page content
• ➖ Requires additional harness/setup (route, fixture, or Storybook-like environment)
• ➖ May drift from real integration context if not kept in sync

Recommendation: The current approach (scoping axe to "#global-header") is the best trade-off for this test: it restores full rule coverage while ensuring the scan only evaluates the header’s DOM. Excluding unrelated regions would be more brittle, and disabling nested-interactive again would reduce confidence in catching real header regressions.

Files changed (2) +14 / -5

Bug fix (1) +13 / -4

accessibility.tsAdd optional Axe include-scope; stop disabling nested-interactive +13/-4

Add optional Axe include-scope; stop disabling nested-interactive

• Extends the accessibility helper with an optional "include" selector and applies it via AxeBuilder.include(). Removes the previous blanket disablement of the nested-interactive rule so scoped scans remain strict.

workspaces/global-header/e2e-tests/utils/accessibility.ts

Tests (1) +1 / -1

globalHeader.test.tsScope global header a11y scan to #global-header +1/-1

Scope global header a11y scan to #global-header

• Updates the global-header visibility test to run accessibility checks only within the "#global-header" element. This prevents unrelated page content (rendered below the header) from causing violations in this test.

workspaces/global-header/e2e-tests/globalHeader.test.ts

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 11 rules

✅ Cross-repo context

  Not relevant to this PR: redhat-developer/rhdh

  Not relevant to this PR: redhat-developer/rhdh-chart

  Not relevant to this PR: redhat-developer/rhdh-operator

  Not relevant to this PR: redhat-developer/rhdh-local

1. Scoped a11y misses dropdowns 🐞 Bug ≡ Correctness

Description

The global-header test now scopes axe to #global-header, so any global-header-controlled UI that
is not a DOM descendant of that element will not be analyzed and can regress without failing CI.
Global header dropdowns render a Menu with an id (${id}-menu) that is controlled by header
buttons, so if that menu mounts outside #global-header (common with menu/portal patterns), its
violations will be skipped.

Code

workspaces/global-header/e2e-tests/globalHeader.test.ts[108]

+  await runAccessibilityTests(page, testInfo, undefined, '#global-header');

Relevance

⭐⭐ Medium

Scoping axe to subtree may miss portal menus; but PR goal is intentional narrowing; no close
precedent found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test now calls runAccessibilityTests(..., '#global-header'), and the helper uses
builder.include(include) which limits analysis to that subtree. The global header dropdown
implementation renders a Menu with a computed ${id}-menu id and ties it to the header button via
aria-controls, meaning dropdown content is part of the header interaction surface and can be
missed if it is not within the included subtree.

workspaces/global-header/e2e-tests/globalHeader.test.ts[83-109]
workspaces/global-header/e2e-tests/utils/accessibility.ts[20-38]
workspaces/global-header/plugins/global-header/src/components/HeaderDropdownComponent/HeaderDropdownComponent.tsx[82-147]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The a11y scan is scoped to `#global-header`, which can exclude header-owned dropdown menu content (and other portal-mounted UI) from the axe analysis, reducing test coverage.

## Issue Context
Header dropdown buttons control a `Menu` element with id `${id}-menu` (and `keepMounted`), which may not be a descendant of `#global-header` in the DOM.

## Fix Focus Areas
- workspaces/global-header/e2e-tests/globalHeader.test.ts[83-109]
- workspaces/global-header/e2e-tests/utils/accessibility.ts[20-38]
- workspaces/global-header/plugins/global-header/src/components/HeaderDropdownComponent/HeaderDropdownComponent.tsx[82-147]

## Suggested fix
1. Verify where the dropdown `Menu` mounts in the DOM during e2e runs.
2. If it is outside `#global-header`, expand the axe scope to include the menu root(s) as well (e.g., support multiple include selectors in the helper and include both the header and the relevant menu container selectors/ids).
3. Optionally add a focused accessibility scan after opening each dropdown to ensure menu content is checked explicitly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Brittle positional include arg 🐞 Bug ⚙ Maintainability

Description

runAccessibilityTests adds include as a 4th positional parameter, forcing call sites to pass
undefined just to use the default attachment name. This increases the chance of accidental misuse
(e.g., a selector passed as the 3rd argument becomes the attachment filename) and makes the helper
harder to extend safely.

Code

workspaces/global-header/e2e-tests/utils/accessibility.ts[R21-37]

  page: Page,
  testInfo: TestInfo,
  attachName = 'accessibility-scan-results.json',
+  include?: string,
) {
-  const accessibilityScanResults = await new AxeBuilder({ page })
-    .withTags(['wcag2a', 'wcag2aa', 'wcag21a', 'wcag21aa'])
-    .disableRules(['nested-interactive'])
-    .analyze();
+  let builder = new AxeBuilder({ page }).withTags([
+    'wcag2a',
+    'wcag2aa',
+    'wcag21a',
+    'wcag21aa',
+  ]);
+
+  if (include) {
+    builder = builder.include(include);
+  }
+
+  const accessibilityScanResults = await builder.analyze();

Relevance

⭐⭐ Medium

Refactoring helper to options object is subjective API change; limited/indirect precedent on
signature tweaks.

PR-#2499

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The updated test must pass undefined as the third argument to reach the new include parameter,
directly demonstrating the brittleness of the new positional API.

workspaces/global-header/e2e-tests/globalHeader.test.ts[96-109]
workspaces/global-header/e2e-tests/utils/accessibility.ts[20-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The helper now requires an `undefined` placeholder to supply `include`, which is brittle and error-prone.

## Issue Context
Call sites that want the default `attachName` but also want to set `include` must call `runAccessibilityTests(page, testInfo, undefined, selector)`.

## Fix Focus Areas
- workspaces/global-header/e2e-tests/utils/accessibility.ts[20-38]
- workspaces/global-header/e2e-tests/globalHeader.test.ts[83-109]

## Suggested fix
Refactor the helper signature to accept an options object, e.g.:
```ts
runAccessibilityTests(page, testInfo, {
 attachName?: string,
 includeSelectors?: string[],
});
```
Then update the call site to:
```ts
await runAccessibilityTests(page, testInfo, { includeSelectors: ['#global-header'] });
```
This also makes it straightforward to include multiple selectors if needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---