fix(a11y): resolve Lighthouse button-name and aria-valid-attr-value failures in output modal - #14278
Merged
Conversation
…ailures in output modal
Contributor
WalkthroughChangesOutputModal behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OutputModal
participant Tabs
participant TabsContent
participant SwitchOutputView
OutputModal->>Tabs: set active tab value
Tabs->>TabsContent: select outputs or logs panel
TabsContent->>SwitchOutputView: render fixed tab type
SwitchOutputView-->>TabsContent: render selected data view
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
bug
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx (1)
27-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExtract the shared outputs/logs selection logic into one helper. Both files independently re-implement
activeTab/type === "outputs" ? data.outputs[...] : data.logs[...]against the sameOutputModalTabcontract; this PR already had to touch both spots to rename the tab literals, showing the duplication is prone to drifting.
src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx#L27-L46: extract a small shared helper (e.g.getFlowPoolResult(flowPoolNode, outputName, tab)) that bothgetOutputContenthere andSwitchOutputViewcan call instead of duplicating the ternary.src/frontend/src/CustomNodes/GenericNode/components/outputModal/components/switchOutputView/index.tsx#L51-L54: replace the localtype === "outputs" ? ... : ...selection with the same shared helper.♻️ Proposed shared helper
// e.g. in a shared utils module colocated with OutputModalTab export function getFlowPoolResult( flowPoolNode: { data?: { outputs?: Record<string, unknown>; logs?: Record<string, unknown> } } | undefined, outputName: string, tab: OutputModalTab, ) { return tab === "outputs" ? flowPoolNode?.data?.outputs?.[outputName] : flowPoolNode?.data?.logs?.[outputName]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx` around lines 27 - 46, Extract the duplicated outputs/logs lookup into a shared getFlowPoolResult helper aligned with the OutputModalTab contract. Update src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx lines 27-46 so getOutputContent uses the helper, and update src/frontend/src/CustomNodes/GenericNode/components/outputModal/components/switchOutputView/index.tsx lines 51-54 so SwitchOutputView uses it instead of its local ternary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx`:
- Around line 27-46: Extract the duplicated outputs/logs lookup into a shared
getFlowPoolResult helper aligned with the OutputModalTab contract. Update
src/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsx lines
27-46 so getOutputContent uses the helper, and update
src/frontend/src/CustomNodes/GenericNode/components/outputModal/components/switchOutputView/index.tsx
lines 51-54 so SwitchOutputView uses it instead of its local ternary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 57a14754-c589-4191-8d92-b25e05b3058d
📒 Files selected for processing (10)
src/frontend/src/CustomNodes/GenericNode/components/outputModal/__tests__/outputModal.a11y.test.tsxsrc/frontend/src/CustomNodes/GenericNode/components/outputModal/components/switchOutputView/index.tsxsrc/frontend/src/CustomNodes/GenericNode/components/outputModal/index.tsxsrc/frontend/src/locales/de.jsonsrc/frontend/src/locales/en.jsonsrc/frontend/src/locales/es.jsonsrc/frontend/src/locales/fr.jsonsrc/frontend/src/locales/ja.jsonsrc/frontend/src/locales/pt.jsonsrc/frontend/src/locales/zh-Hans.json
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (16.27%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14278 +/- ##
==================================================
+ Coverage 61.26% 61.39% +0.12%
==================================================
Files 2347 2344 -3
Lines 238587 236781 -1806
Branches 35590 33317 -2273
==================================================
- Hits 146180 145378 -802
+ Misses 90585 89636 -949
+ Partials 1822 1767 -55
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
viktoravelino
approved these changes
Jul 28, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two Lighthouse accessibility failures on the component output modal (Outputs/Logs tabs + copy-to-clipboard button):
button-name— the icon-only copy button had no accessible name.aria-valid-attr-value— the tab switcher'saria-controlspointed at a panel element that never existed.What changed
aria-labelto the copy button, dynamically reflecting the active tab — "Copy output" on the Outputs tab, "Copy logs" on the Logs tab — since the button copies whatever tab is currently showing.TabsTriggerwith a matchingTabsContent(Radix's expected trigger/content contract). Previously the panel content (SwitchOutputView) was a disconnected sibling driven by manual string branching, so Radix's auto-generatedaria-controlsreferenced a nonexistent id."Outputs"/"Logs"→ lowercase"outputs"/"logs", decoupled from the translated visible labels. Extracted as an exportedOutputModalTabtype shared betweenoutputModal/index.tsxandswitchOutputView/index.tsxinstead of duplicating the union.overflow-hidden, absolutely-positioned pill meant only for the switcher buttons. Fixed by scoping that constrained styling toTabsListalone and givingTabs/TabsContenta properflex-1chain so the content fills the modal again.TabsList'soverflow-hiddenwas clipping the browser's default focus outline on the tab triggers. Replaced with an inset focus ring (focus-visible:ring-2 focus-visible:ring-inset), which renders inside the trigger's own box and can't be clipped.output.copyOutputAriaandoutput.copyLogsAriaacross all 7 locales (en, de, es, fr, ja, pt, zh-Hans).outputModal.a11y.test.tsx(10 tests) — axe checks, accessible-name correctness per tab,aria-controlsvalidity, tab-switch/copy behavior parity, and regression guards for the layout and focus-ring fixes.How QA can test
Automated
cd src/frontend
npx jest src/CustomNodes/GenericNode/components/outputModal --no-coverage
npx tsc --noEmit
Expect all 10 tests passing, no new type errors.
Manual — copy button naming
Cmd+F5) and Tab to the copy button.Manual — tab switcher keyboard behavior
Tabagain — focus should leave the tablist entirely (move to the panel content), not jump to "Logs". This is expected per the ARIA tabs pattern.Left/Rightarrow keys — focus and selection should move between Outputs and Logs.Manual — layout
Manual — translations
Summary by CodeRabbit
Accessibility
Bug Fixes
Localization
Tests