fix(app): remove SVG prop and nested button warnings on web (#7915)

* fix(app): prevent web SVG prop and nested button warnings

Consume Unistyles mapping props at the wrapper boundary and use web accessibility attributes for decorative rings. Register composer track rows as menu items so nested actions stay valid and keyboard selection remains available.

* test(app): select archive actions by their menu role
This commit is contained in:
Mohamed Boudra
2026-10-26 15:06:02 +07:00
committed by GitHub
parent 033b2cdfc2
commit e958540b0a
9 changed files with 293 additions and 9 deletions
@@ -42,6 +42,14 @@ const test = base.extend<{ agent: MockAgentSession }>({
},
});
// The outer meter owns the accessible usage label; its SVG is decorative.
async function expectDecorativeContextRing(meter: Locator): Promise<void> {
const ring = meter.locator("svg");
await expect(ring).toHaveAttribute("aria-hidden", "true");
await expect(ring).not.toHaveAttribute("accessibilityElementsHidden");
await expect(ring).not.toHaveAttribute("importantForAccessibility");
}
// Where the progress arc is painted, as its centroid relative to the ring's centre in pixels.
// Reads the rendered pixels, so any rotation that does not reach the screen counts as none.
async function progressArcCentroid(meter: Locator): Promise<{ x: number; y: number }> {
@@ -100,6 +108,7 @@ test.describe("context window meter", () => {
const meter = page.getByTestId("context-window-meter");
await expect(meter).toHaveAccessibleName(/25%/, { timeout: 30_000 });
await expectDecorativeContextRing(meter);
const centroid = await progressArcCentroid(meter);
expect(centroid.x).toBeGreaterThan(1);
expect(centroid.y).toBeLessThan(-1);
@@ -188,7 +188,7 @@ export async function expectManagedSubagentArchived(
}
export async function archiveFinishedSubagents(page: Page): Promise<void> {
await page.getByRole("button", { name: "Archive finished subagents" }).click();
await page.getByRole("menuitem", { name: "Archive finished subagents" }).click();
}
export async function expectArchiveFinishedInProgress(
@@ -196,7 +196,7 @@ export async function expectArchiveFinishedInProgress(
completed: number,
total: number,
): Promise<void> {
await expect(page.getByRole("button", { name: "Archive finished subagents" })).toBeDisabled();
await expect(page.getByRole("menuitem", { name: "Archive finished subagents" })).toBeDisabled();
await expect(page.getByText(`${completed}/${total}`, { exact: true })).toBeVisible();
}
@@ -206,7 +206,7 @@ export async function expectArchiveFinishedRetry(
total: number,
): Promise<void> {
await expect(page.getByText(`Retry (${failed}/${total})`, { exact: true })).toBeVisible();
await expect(page.getByRole("button", { name: "Archive finished subagents" })).toBeEnabled();
await expect(page.getByRole("menuitem", { name: "Archive finished subagents" })).toBeEnabled();
}
export async function holdManagedSubagentArchiveRequest(
@@ -112,8 +112,9 @@ const ContextWindowRing = withUnistyles(function ContextWindowRing({
width={size}
height={size}
viewBox={`0 0 ${size} ${size}`}
accessibilityElementsHidden
importantForAccessibility="no-hide-descendants"
{...(isNative
? { accessibilityElementsHidden: true, importantForAccessibility: "no-hide-descendants" }
: { "aria-hidden": true })}
>
<Circle
cx={center}
@@ -162,12 +162,34 @@ describe("composer track panel", () => {
);
openPanel();
expect(document.querySelector("button button")).toBeNull();
expect(row("row")?.getAttribute("role")).toBe("menuitem");
click(row("row-archive") as Element);
expect(onAction).toHaveBeenCalledTimes(1);
expect(onPress).not.toHaveBeenCalled();
expect(isPanelOpen()).toBe(true);
});
it.each(["Enter", " "])("selects a track row with %j", (key) => {
const select = vi.fn();
mount(
<ComposerTrackPill testID="pill" segments={SUBAGENT_SEGMENTS} panelTitle="Subagents">
<ComposerTrackRow accessibilityLabel="Subagent one" testID="row" onPress={select}>
<Text>Subagent one</Text>
</ComposerTrackRow>
</ComposerTrackPill>,
);
openPanel();
const target = row("row") as HTMLElement;
act(() => {
target.focus();
target.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true }));
target.dispatchEvent(new KeyboardEvent("keyup", { key, bubbles: true }));
});
expect(select).toHaveBeenCalledTimes(1);
expect(isPanelOpen()).toBe(false);
});
});
describe("composer track pill status mark", () => {
+7 -1
View File
@@ -233,6 +233,11 @@ export function ComposerTrackRow({
[closeOnSelect, onPress, selectItem],
);
const itemDataSet = useMemo(
() => ({ menuItem: "true", menuDisabled: disabled ? "true" : "false" }),
[disabled],
);
const renderRow = useCallback(
(active: boolean) => (
<View style={active ? styles.rowActive : styles.row}>
@@ -253,7 +258,8 @@ export function ComposerTrackRow({
return (
<View onPointerEnter={handlePointerEnter} onPointerLeave={handlePointerLeave}>
<Pressable
accessibilityRole="button"
accessibilityRole="menuitem"
dataSet={itemDataSet}
accessibilityLabel={accessibilityLabel}
testID={testID}
disabled={disabled}
@@ -1,3 +1,4 @@
import { isNative } from "@/constants/platform";
import Svg, { Circle } from "react-native-svg";
import { StyleSheet, withUnistyles } from "react-native-unistyles";
import type { Theme } from "@/styles/theme";
@@ -75,8 +76,9 @@ function ChecksRingSvg({
height={size}
viewBox={`0 0 ${VIEW_BOX} ${VIEW_BOX}`}
style={ringStyles.svg}
accessibilityElementsHidden
importantForAccessibility="no-hide-descendants"
{...(isNative
? { accessibilityElementsHidden: true, importantForAccessibility: "no-hide-descendants" }
: { "aria-hidden": true })}
>
<Circle
cx={CENTER}
@@ -0,0 +1,86 @@
import { spawnSync } from "node:child_process";
import path from "node:path";
import { build } from "esbuild";
import { describe, expect, it } from "vitest";
// Bypass the app test aliases: they replace all three libraries involved in the leak.
async function renderWebProbe(contents: string) {
const { outputFiles } = await build({
stdin: {
contents,
resolveDir: path.resolve(import.meta.dirname, "../.."),
loader: "jsx",
},
bundle: true,
platform: "node",
format: "cjs",
write: false,
mainFields: ["browser", "module", "main"],
conditions: ["browser"],
resolveExtensions: [".web.tsx", ".web.ts", ".web.js", ".tsx", ".ts", ".js", ".json"],
alias: { "react-native": "react-native-web", "@": path.resolve(import.meta.dirname, "..") },
jsx: "automatic",
define: { __DEV__: "true", "process.env.NODE_ENV": '"development"' },
});
return spawnSync(process.execPath, ["-"], {
input: outputFiles[0].text,
encoding: "utf8",
});
}
describe("Unistyles web component props", () => {
it("consumes uniProps while preserving mapped colors and explicit overrides", async () => {
const result = await renderWebProbe(`
import React from 'react';
import { renderToStaticMarkup } from 'react-dom/server';
import { StyleSheet, UnistylesRuntime, withUnistyles } from 'react-native-unistyles';
import { Server } from 'lucide-react-native';
StyleSheet.configure({
themes: { light: { color: 'red' }, dark: { color: 'blue' } },
settings: { initialTheme: 'light' },
});
const Icon = withUnistyles(Server);
const mapping = theme => ({ color: theme.color });
const render = props => renderToStaticMarkup(React.createElement(Icon, { uniProps: mapping, ...props }));
const light = render();
UnistylesRuntime.setTheme('dark');
const dark = render();
const overridden = render({ color: 'green' });
const received = [];
const Probe = withUnistyles(props => { received.push(props); return null; });
renderToStaticMarkup(React.createElement(Probe, { uniProps: mapping, id: 'probe' }));
console.log(JSON.stringify({ light, dark, overridden, keys: Object.keys(received[0]).sort() }));
`);
expect(result.status, result.stderr).toBe(0);
expect(result.stderr).not.toContain("React does not recognize");
const rendered = JSON.parse(result.stdout);
expect(rendered.keys).toEqual(["color", "id", "ref"]);
expect(rendered.light).toContain('stroke="red"');
expect(rendered.dark).toContain('stroke="blue"');
expect(rendered.overridden).toContain('stroke="green"');
});
});
describe("decorative checks ring", () => {
it("hides its SVG from web accessibility without forwarding native props", async () => {
const result = await renderWebProbe(`
import React from 'react';
import { renderToStaticMarkup } from 'react-dom/server';
import { StyleSheet } from 'react-native-unistyles';
import { ChecksRing } from './src/git/pull-request-panel/checks-ring';
StyleSheet.configure({
themes: { light: { colors: { surface3: 'gray', statusSuccess: 'green' } } },
settings: { initialTheme: 'light' },
});
console.log(renderToStaticMarkup(React.createElement(ChecksRing, {
size: 20, summary: { total: 1, parts: [{ status: 'success', count: 1 }] },
})));
`);
expect(result.status, result.stderr).toBe(0);
expect(result.stdout).toContain('aria-hidden="true"');
expect(result.stdout).not.toContain("importantForAccessibility");
expect(result.stdout).not.toContain("accessibilityElementsHidden");
expect(result.stdout).toContain('stroke="green"');
expect(result.stderr).not.toContain("React does not recognize");
});
});
+157
View File
@@ -329,3 +329,160 @@ index 195954b..c4ac28c 100644
namespace "com.unistyles"
defaultConfig {
diff --git a/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.tsx b/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.tsx
--- a/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.tsx
+++ b/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.tsx
@@ -28,13 +28,14 @@
return forwardRef<GenericComponentRef<TComponent>, PropsWithUnistyles>((props, ref) => {
const narrowedProps = props as PropsWithUnistyles & UnistyleStyles
+ const { uniProps, ...componentProps } = narrowedProps
const styleClassNames = getClassName(narrowedProps.style, true)
const contentContainerStyleClassNames = getClassName(narrowedProps.contentContainerStyle)
const { proxifiedRuntime, proxifiedTheme } = useProxifiedUnistyles()
const { key: mappingsKey, ...mappingsProps } = mappings ? mappings(proxifiedTheme, proxifiedRuntime) : {}
- const { key: uniPropsKey, ...unistyleProps } = narrowedProps.uniProps
- ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime)
+ const { key: uniPropsKey, ...unistyleProps } = uniProps
+ ? uniProps(proxifiedTheme, proxifiedRuntime)
: {}
const emptyStyles = narrowedProps.style
@@ -46,7 +47,7 @@
: undefined
const combinedProps = {
- ...deepMergeObjects(mappingsProps, unistyleProps, props),
+ ...deepMergeObjects(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style
? {
// Override default component styles with undefined values to reset them
diff --git a/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.native.tsx b/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.native.tsx
--- a/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.native.tsx
+++ b/node_modules/react-native-unistyles/src/core/withUnistyles/withUnistyles.native.tsx
@@ -63,6 +63,7 @@
return forwardRef<GenericComponentRef<TComponent>, PropsWithUnistyles>((props, ref) => {
const narrowedProps = props as PropsWithUnistyles & UnistyleStyles
+ const { uniProps, ...componentProps } = narrowedProps
const NativeComponent = Component as ComponentType
// @ts-ignore we don't know the type of the component
@@ -89,15 +90,15 @@
}, [narrowedProps.style, narrowedProps.contentContainerStyle])
const { key: mappingsKey, ...mappingsProps } = mappings ? mappings(proxifiedTheme, proxifiedRuntime) : {}
- const { key: uniPropsKey, ...unistyleProps } = narrowedProps.uniProps
- ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime)
+ const { key: uniPropsKey, ...unistyleProps } = uniProps
+ ? uniProps(proxifiedTheme, proxifiedRuntime)
: {}
const styleSecrets = getSecrets(narrowedProps.style)
const contentContainerStyleSecrets = getSecrets(narrowedProps.contentContainerStyle)
const finalProps = {
- ...deepMergeObjects(mappingsProps, unistyleProps, props),
+ ...deepMergeObjects(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style
? {
style: styleSecrets.styles,
diff --git a/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.js b/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.js
--- a/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.js
+++ b/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.js
@@ -13,6 +13,7 @@
export const withUnistyles = (Component, mappings) => {
return /*#__PURE__*/forwardRef((props, ref) => {
const narrowedProps = props;
+ const { uniProps, ...componentProps } = narrowedProps;
const styleClassNames = getClassName(narrowedProps.style, true);
const contentContainerStyleClassNames = getClassName(narrowedProps.contentContainerStyle);
const {
@@ -26,10 +27,10 @@
const {
key: uniPropsKey,
...unistyleProps
- } = narrowedProps.uniProps ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime) : {};
+ } = uniProps ? uniProps(proxifiedTheme, proxifiedRuntime) : {};
const emptyStyles = narrowedProps.style ? Object.fromEntries(Object.entries(Object.getOwnPropertyDescriptors(narrowedProps.style)).filter(([key]) => !key.startsWith('unistyles') && !key.startsWith('_')).map(([key]) => [key, undefined])) : undefined;
const combinedProps = {
- ...deepMergeObjects(mappingsProps, unistyleProps, props),
+ ...deepMergeObjects(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style ? {
// Override default component styles with undefined values to reset them
style: emptyStyles
diff --git a/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.native.js b/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.native.js
--- a/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.native.js
+++ b/node_modules/react-native-unistyles/lib/module/core/withUnistyles/withUnistyles.native.js
@@ -27,6 +27,7 @@
};
return /*#__PURE__*/forwardRef((props, ref) => {
const narrowedProps = props;
+ const { uniProps, ...componentProps } = narrowedProps;
const NativeComponent = Component;
// @ts-ignore we don't know the type of the component
@@ -57,11 +58,11 @@
const {
key: uniPropsKey,
...unistyleProps
- } = narrowedProps.uniProps ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime) : {};
+ } = uniProps ? uniProps(proxifiedTheme, proxifiedRuntime) : {};
const styleSecrets = getSecrets(narrowedProps.style);
const contentContainerStyleSecrets = getSecrets(narrowedProps.contentContainerStyle);
const finalProps = {
- ...deepMergeObjects(mappingsProps, unistyleProps, props),
+ ...deepMergeObjects(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style ? {
style: styleSecrets.styles
} : {}),
diff --git a/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.js b/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.js
--- a/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.js
+++ b/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.js
@@ -18,6 +18,7 @@
const withUnistyles = (Component, mappings) => {
return /*#__PURE__*/(0, _react.forwardRef)((props, ref) => {
const narrowedProps = props;
+ const { uniProps, ...componentProps } = narrowedProps;
const styleClassNames = (0, _getClassname.getClassName)(narrowedProps.style, true);
const contentContainerStyleClassNames = (0, _getClassname.getClassName)(narrowedProps.contentContainerStyle);
const {
@@ -31,10 +32,10 @@
const {
key: uniPropsKey,
...unistyleProps
- } = narrowedProps.uniProps ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime) : {};
+ } = uniProps ? uniProps(proxifiedTheme, proxifiedRuntime) : {};
const emptyStyles = narrowedProps.style ? Object.fromEntries(Object.entries(Object.getOwnPropertyDescriptors(narrowedProps.style)).filter(([key]) => !key.startsWith('unistyles') && !key.startsWith('_')).map(([key]) => [key, undefined])) : undefined;
const combinedProps = {
- ...(0, _utils.deepMergeObjects)(mappingsProps, unistyleProps, props),
+ ...(0, _utils.deepMergeObjects)(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style ? {
// Override default component styles with undefined values to reset them
style: emptyStyles
diff --git a/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.native.js b/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.native.js
--- a/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.native.js
+++ b/node_modules/react-native-unistyles/lib/commonjs/core/withUnistyles/withUnistyles.native.js
@@ -32,6 +32,7 @@
};
return /*#__PURE__*/(0, _react.forwardRef)((props, ref) => {
const narrowedProps = props;
+ const { uniProps, ...componentProps } = narrowedProps;
const NativeComponent = Component;
// @ts-ignore we don't know the type of the component
@@ -62,11 +63,11 @@
const {
key: uniPropsKey,
...unistyleProps
- } = narrowedProps.uniProps ? narrowedProps.uniProps(proxifiedTheme, proxifiedRuntime) : {};
+ } = uniProps ? uniProps(proxifiedTheme, proxifiedRuntime) : {};
const styleSecrets = getSecrets(narrowedProps.style);
const contentContainerStyleSecrets = getSecrets(narrowedProps.contentContainerStyle);
const finalProps = {
- ...(0, _utils.deepMergeObjects)(mappingsProps, unistyleProps, props),
+ ...(0, _utils.deepMergeObjects)(mappingsProps, unistyleProps, componentProps),
...(narrowedProps.style ? {
style: styleSecrets.styles
} : {}),
+2 -1
View File
@@ -19,8 +19,9 @@ const patchedPackages = [
patchPrefix: "react-native-enriched-markdown+",
cwd: "packages/app",
},
// Remove after react-native-unistyles ships
// Registry cleanup: remove after react-native-unistyles ships
// https://github.com/jpudysz/react-native-unistyles/pull/1203.
// withUnistyles: also keep the uniProps filter until upstream consumes its control prop.
{
nodeModulesPath: "node_modules/react-native-unistyles",
patchPrefix: "react-native-unistyles+",