Skip to content

fix: UI audit Chunk 5 — low severity token and a11y fixes #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joshuapare merged 5 commits into from
Mar 11, 2026
Merged

fix: UI audit Chunk 5 — low severity token and a11y fixes #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
868f05f
fix(tokens): replace hardcoded font-size, z-index, and opacity with t…
joshuapare Mar 11, 2026
1c93965
fix(a11y): add keyboard support to RowList sortable column headers
joshuapare Mar 11, 2026
1a6e34f
docs: regenerate audit findings report after Chunk 5 fixes
joshuapare Mar 11, 2026
126cd89
fix: address first round of review comments for PR #24
joshuapare Mar 11, 2026
238a181
fix: gate RowList data-sortable on onSortChange presence
joshuapare Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,280 changes: 1,314 additions & 966 deletions docs/superpowers/specs/audit-findings.md
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/ai-ui/src/components/agent/AgentStatusItem.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
flex-shrink: 0;
width: 16px;
text-align: center;
font-size: 12px;
font-size: var(--ov-font-size-caption, 0.75rem);
line-height: 1;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/ai-ui/src/components/artifact/AIArtifact.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

.Title {
margin: 0;
font-size: 0.875rem;
font-size: var(--ov-font-size-body);
font-weight: 600;
line-height: 1.4;
color: var(--ov-color-text-primary);
Expand All @@ -44,7 +44,7 @@

.Description {
margin: 0;
font-size: 0.75rem;
font-size: var(--ov-font-size-caption);
line-height: 1.4;
color: var(--ov-color-text-secondary);
overflow: hidden;
Expand Down
2 changes: 1 addition & 1 deletion packages/ai-ui/src/components/branching/AIBranch.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
}

.Indicator {
font-size: 0.75rem;
font-size: var(--ov-font-size-caption);
color: var(--ov-palette-text-secondary, #6b7280);
font-variant-numeric: tabular-nums;
user-select: none;
Expand Down
2 changes: 1 addition & 1 deletion packages/ai-ui/src/components/content/AICodeBlock.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
border-radius: var(--ov-radius-sm, 3px);
background: transparent;
color: var(--ov-color-fg-muted);
font-size: 14px;
font-size: var(--ov-font-size-body, 0.875rem);
cursor: pointer;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ai-ui/src/components/tool-calls/ToolCall.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

.StatusIcon {
flex-shrink: 0;
font-size: 12px;
font-size: var(--ov-font-size-caption, 0.75rem);
line-height: 1;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/accordion/Accordion.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
}

.Item[data-ov-disabled='true'] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

.Item[data-ov-disabled='true'] > .Header {
Expand Down
4 changes: 2 additions & 2 deletions packages/base-ui/src/components/alert-dialog/AlertDialog.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.Backdrop {
position: fixed;
inset: 0;
z-index: 1000;
z-index: var(--ov-z-modal, 1000);
background: color-mix(in srgb, var(--ov-color-bg-base) 38%, transparent 62%);
backdrop-filter: blur(1px);
}
Expand All @@ -15,7 +15,7 @@

display: grid;
position: relative;
z-index: 1001;
z-index: calc(var(--ov-z-modal, 1000) + 1);
gap: var(--ov-space-stack-md);
min-inline-size: min(320px, calc(100vw - (var(--ov-space-stack-lg) * 2)));
max-inline-size: min(92vw, 480px);
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/autocomplete/Autocomplete.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/button/Button.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
}

.Root:is([disabled], [data-disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}
Comment on lines 51 to 54
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for --ov-opacity-disabled definition in the codebase
rg -n '--ov-opacity-disabled\s*:' --type css

Repository: omniviewdev/ui

Length of output: 485

🏁 Script executed:

#!/bin/bash
# Search for --ov-opacity-disabled definition (correct syntax)
echo "=== Searching for --ov-opacity-disabled definition ==="
rg '\-\-ov-opacity-disabled\s*:' --type css

echo ""
echo "=== Searching for --ov-opacity-disabled usage ==="
rg '\-\-ov-opacity-disabled' --type css

echo ""
echo "=== Searching for CSS variable patterns with fallbacks ==="
rg 'var\([^,)]+,' --type css | head -20

echo ""
echo "=== Checking Button.module.css for context ==="
cat packages/base-ui/src/components/button/Button.module.css 2>/dev/null || echo "File not found, searching for it..."

Repository: omniviewdev/ui

Length of output: 10569

Add fallback value for --ov-opacity-disabled in 23 components.

The CSS variable --ov-opacity-disabled is not defined anywhere in the codebase. Without a fallback, the opacity property will be invalid, causing the disabled state styling to fail across all affected components. This inconsistency is evident since ChatInput.module.css already includes a fallback (var(--ov-opacity-disabled, 0.5)), while 22 other components lack one.

Affected files:

  • Button.module.css
  • Checkbox.module.css
  • Input.module.css
  • TextField.module.css
  • And 19 others
♻️ Suggested fix 
 .Root:is([disabled], [data-disabled]) {
-  opacity: var(--ov-opacity-disabled);
+  opacity: var(--ov-opacity-disabled, 0.5);
   cursor: not-allowed;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.Root:is([disabled], [data-disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled);
cursor: not-allowed;
}
.Root:is([disabled], [data-disabled]) {
opacity: var(--ov-opacity-disabled, 0.5);
cursor: not-allowed;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/base-ui/src/components/button/Button.module.css` around lines 51 -
54, The disabled-state rule ".Root:is([disabled], [data-disabled])" uses the
undefined CSS variable --ov-opacity-disabled; update this declaration to include
a safe fallback (e.g., change opacity: var(--ov-opacity-disabled) to opacity:
var(--ov-opacity-disabled, 0.5)) in Button.module.css and apply the same
fallback pattern to the same selector in the other 22 affected component CSS
files (Checkbox.module.css, Input.module.css, TextField.module.css, etc.) so the
disabled styling remains valid when the variable is not set.


Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/checkbox/Checkbox.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
}

.Root:is([data-disabled], [disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/chip/Chip.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@
}

.Root:is([disabled], [data-disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/collapsible/Collapsible.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

.Trigger:disabled {
cursor: default;
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
}

.Trigger:focus-visible {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/combobox/Combobox.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/command-list/CommandList.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@
}

.Item[data-ov-disabled='true'] {
opacity: var(--ov-opacity-disabled);
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/confirm-button/ConfirmButton.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
}

.Root:disabled {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/context-menu/ContextMenu.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/dialog/Dialog.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
.Overlay {
position: fixed;
inset: 0;
z-index: 9000;
z-index: var(--ov-z-modal, 9000);
display: flex;
align-items: center;
justify-content: center;
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/editor-tabs/EditorTabs.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@
}

.Tab[data-disabled] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
cursor: default;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/input/Input.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
}

.ControlShell[data-disabled] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/list/List.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
}

.Item[data-ov-disabled='true'] {
opacity: var(--ov-opacity-disabled);
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
cursor: default;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/menu/Menu.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/multi-select/MultiSelect.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/number-input/NumberInput.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
}

.Group[data-disabled] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/pagination/Pagination.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
/* Disabled (prev on first page, next on last page – excludes active page) */

.Item:disabled:not([data-ov-active='true']) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/base-ui/src/components/popover/Popover.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.Portal {
z-index: 150;
z-index: var(--ov-z-popover, 150);
}

.Backdrop {
Expand All @@ -10,7 +10,7 @@
}

.Positioner {
z-index: 151;
z-index: calc(var(--ov-z-popover, 150) + 1);
}

.Trigger,
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/radio/Radio.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
}

.Root:is([data-disabled], [disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
Comment on lines 122 to 124
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Definitions of --ov-opacity-disabled:"
rg -nP -- '--ov-opacity-disabled\s*:' || true

echo
echo "Usages of --ov-opacity-disabled:"
rg -nP -- 'var\(--ov-opacity-disabled\b'

Repository: omniviewdev/ui

Length of output: 2546

Add fallback values to --ov-opacity-disabled across all disabled-state declarations.

The --ov-opacity-disabled token is not defined anywhere in the codebase. Without a fallback, disabled controls across 22 component modules will render fully opaque, creating accessibility and UX issues. The ChatInput.module.css already sets a fallback of 0.5—establish this as the standard pattern across all components.

Affected files:

  • packages/base-ui/src/components/{button,checkbox,radio,input,...}/[Component].module.css and 19 others

For the Radio component specifically:

.Root:is([data-disabled], [disabled]) {
- opacity: var(--ov-opacity-disabled);
+ opacity: var(--ov-opacity-disabled, 0.5);
  cursor: not-allowed;
}

Apply the same fix to all other disabled-state declarations in the base-ui suite.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/base-ui/src/components/radio/Radio.module.css` around lines 122 -
124, The disabled-state CSS uses the custom property --ov-opacity-disabled
without a fallback, so update the selector .Root:is([data-disabled], [disabled])
in Radio.module.css to supply a fallback (use 0.5 like ChatInput.module.css) and
mirror this change across all other base-ui component CSS files that set opacity
from --ov-opacity-disabled; locate every rule that references
--ov-opacity-disabled (e.g., opacity: var(--ov-opacity-disabled)) and change it
to opacity: var(--ov-opacity-disabled, 0.5) to ensure consistent disabled
opacity.

}

Expand Down
19 changes: 16 additions & 3 deletions packages/base-ui/src/components/row-list/RowList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createContext, forwardRef, useCallback, useContext, useMemo, type CSSProperties } from 'react';
import { createContext, forwardRef, useCallback, useContext, useMemo, type CSSProperties, type KeyboardEvent } from 'react';
import { LuArrowUp, LuArrowDown } from 'react-icons/lu';
import { cn } from '../../system/classnames';
import { List } from '../list';
Expand Down Expand Up @@ -90,6 +90,17 @@ const RowListHeader = forwardRef<HTMLDivElement, RowListHeaderProps>(function Ro
[sortState, onSortChange],
);

const handleHeaderKeyDown = useCallback(
(col: ColumnDef, e: KeyboardEvent<HTMLDivElement>) => {
if (!col.sortable) return;
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
handleHeaderClick(col);
}
},
[handleHeaderClick],
);

return (
<div ref={ref} role="row" className={cn(styles.Header, className)} {...props}>
{columns.map((col) => {
Expand All @@ -102,14 +113,16 @@ const RowListHeader = forwardRef<HTMLDivElement, RowListHeaderProps>(function Ro
key={col.id}
role="columnheader"
className={styles.HeaderCell}
data-sortable={col.sortable ? '' : undefined}
data-sortable={col.sortable && onSortChange ? '' : undefined}
aria-sort={ariaSortValue}
tabIndex={col.sortable && onSortChange ? 0 : undefined}
style={
{
'--_row-header-cell-justify': col.align ?? undefined,
} as CSSProperties
}
onClick={col.sortable ? () => handleHeaderClick(col) : undefined}
onClick={col.sortable && onSortChange ? () => handleHeaderClick(col) : undefined}
onKeyDown={col.sortable && onSortChange ? (e) => handleHeaderKeyDown(col, e) : undefined}
>
{col.header}
{col.sortable && isSorted && <SortIconComponent className={styles.SortIcon} />}
Expand Down
4 changes: 2 additions & 2 deletions packages/base-ui/src/components/select/Select.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
}

.Trigger[data-disabled] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down Expand Up @@ -146,7 +146,7 @@
}

.Positioner {
z-index: 120;
z-index: var(--ov-z-popup, 120);
}

.Popup {
Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/selectable-list/SelectableList.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@

/* Disabled state */
.Item[data-ov-disabled='true'] .IndicatorWrapper {
opacity: var(--ov-opacity-disabled);
opacity: var(--ov-opacity-disabled, 0.45);
}

/* Indicator icon sizing */
Expand Down
4 changes: 2 additions & 2 deletions packages/base-ui/src/components/split-button/SplitButton.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
}

.Root[data-ov-disabled='true'] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down Expand Up @@ -67,7 +67,7 @@

.Action:disabled,
.MenuTrigger:disabled {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/stepper/Stepper.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
}

.Step[data-ov-disabled] {
opacity: 0.48;
opacity: var(--ov-opacity-disabled, 0.45);
pointer-events: none;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/tag-input/TagInput.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
}

.Root[data-ov-disabled='true'] {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/text-area/TextArea.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
}

.Control:is([disabled], [data-disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/text-field/TextField.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
}

.Control:is([disabled], [data-disabled]) {
opacity: 0.45;
opacity: var(--ov-opacity-disabled, 0.45);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/base-ui/src/components/toast/Toast.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

.Container {
position: fixed;
z-index: 9999;
z-index: var(--ov-z-toast, 9999);
display: flex;
flex-direction: column;
gap: var(--ov-space-stack-sm);
Expand Down
Loading