Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/shadcn-close-label-i18n.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/components': patch
---

The close button that the `Sheet` and `Dialog` primitives auto-render now announces itself in the session locale instead of always in English. Both buttons are icon-only (a lucide `X`), so their `sr-only` span is not decoration — it IS the control's accessible name, and upstream Shadcn ships it as a hardcoded English literal. Under zh/ja/es every drawer and modal in the console (~20 `SheetContent` consumers plus every `DialogContent` consumer — ChatDock, ActivityFeed, metadata-admin, AiChatPage, BuildDebugDrawer, PeoplePicker, RecordDetailDrawer, …) announced "Close" to a screen reader. The span now renders `<CloseSrLabel />`, which resolves `common.close` (present in all ten locale packs since objectstack#5430) and falls back to English when no `I18nProvider` is mounted, so existing suites and e2e specs that address these controls by their English name are unaffected (objectstack#5505).

Because `packages/components/src/ui/**` is regenerated from the Shadcn registry, the edit is not a hand patch that the next `pnpm shadcn:update` would silently revert: it is declared as data in `scripts/shadcn-local-patches.mjs`, re-applied automatically on every sync (including `--force`), and enforced in both directions — `pnpm shadcn:check` now exits non-zero if a declared patch is missing from the file on disk or can no longer be re-applied to current upstream, and an offline test gates the same invariant on every PR.
5 changes: 3 additions & 2 deletions packages/components/shadcn-components.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@
"dependencies": [
"radix-ui"
],
"registryDependencies": []
"registryDependencies": [],
"localEdits": "objectstack#5505 i18n close patch: `DialogContent` auto-renders an icon-only close button whose `sr-only` span IS its accessible name, and upstream hardcodes the English literal — so under zh/ja/es every modal in the console announced \"Close\" in English. The span is replaced by `<CloseSrLabel />` (`packages/components/src/lib/close-label.tsx`), which resolves `common.close` for the session locale and falls back to English with no I18nProvider mounted. The edit is DECLARED as data in `scripts/shadcn-local-patches.mjs` and re-applied on every `--update`, and `pnpm shadcn:check` exits non-zero if it is ever missing from the file or can no longer be re-applied to upstream."
},
"drawer": {
"source": "https://ui.shadcn.com/r/styles/default/drawer.json",
Expand Down Expand Up @@ -268,7 +269,7 @@
"radix-ui"
],
"registryDependencies": [],
"localEdits": "Adds a `hideOverlay` prop that suppresses `<SheetOverlay />`, for sheets that must not dim the page behind them. Upstream renders the overlay unconditionally."
"localEdits": "Adds a `hideOverlay` prop that suppresses `<SheetOverlay />`, for sheets that must not dim the page behind them. Upstream renders the overlay unconditionally. Also carries the objectstack#5505 i18n close patch (see `dialog` for the full rationale): `SheetContent`'s auto-rendered close button renders `<CloseSrLabel />` instead of a hardcoded English `sr-only` span. Declared in `scripts/shadcn-local-patches.mjs` and RE-APPLIED automatically by every sync, so unlike the `hideOverlay` edit above it does not depend on anyone remembering it."
},
"sidebar": {
"source": "https://ui.shadcn.com/r/styles/default/sidebar.json",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,21 @@ afterEach(() => cleanup());
/**
* Addressing the drawer's close button: by `title`, NOT by role+name.
*
* The shadcn `Sheet` primitive auto-renders a close button of its own, whose
* only accessible name is a hardcoded English `sr-only` span
* (`packages/components/src/ui/sheet.tsx:80` — an upstream No-Touch zone,
* AGENTS.md #7). `NavigationOverlay` CSS-hides it with
* `[&>button:last-of-type]:hidden`, so a real browser drops it from the
* accessibility tree — but jsdom does not apply Tailwind, so RTL still sees it
* and `getByRole('button', { name: 'Close' })` matches two elements under `en`.
* The shadcn `Sheet` primitive auto-renders a close button of its own.
* `NavigationOverlay` CSS-hides it with `[&>button:last-of-type]:hidden`, so a
* real browser drops it from the accessibility tree — but jsdom does not apply
* Tailwind, so RTL still sees it and role+name queries match TWO elements.
*
* Ours is the only close carrying a `title`, so that is the precise handle. The
* primitive's own untranslated label is a separate, out-of-scope finding.
* Ours is the only close carrying a `title`, so that is the precise handle.
*
* As of objectstack#5505 the primitive's own label is translated too (it
* renders `<CloseSrLabel />` rather than a hardcoded English span), so the
* duplicate now appears under EVERY locale rather than only under `en` — under
* `zh` both buttons are named 关闭. The bare `getByRole` assertions that used
* to work here therefore became "found multiple elements" errors, and are now
* written as `getAllByRole(...)` containment checks against our titled button.
* That keeps the original intent — our control is reachable by role + name —
* without asserting anything about how many close buttons jsdom can see.
*/
describe('NavigationOverlay drawer close — accessible name (objectstack#5430)', () => {
it('still reads English under an en session', () => {
Expand All @@ -90,18 +95,20 @@ describe('NavigationOverlay drawer close — accessible name (objectstack#5430)'
it('reads the zh bundle value under a zh session', () => {
renderDrawerIn('zh');

expect(screen.getByTitle('关闭').getAttribute('aria-label')).toBe('关闭');
expect(screen.getByRole('button', { name: '关闭' })).toBeTruthy();
const ours = screen.getByTitle('关闭');
expect(ours.getAttribute('aria-label')).toBe('关闭');
expect(screen.getAllByRole('button', { name: '关闭' })).toContain(ours);
// The literal this replaced, scoped to OUR button via `title` so the
// primitive's hidden English one cannot mask a re-inlined string here.
// primitive's own close cannot mask a re-inlined string here.
expect(screen.queryByTitle('Close')).toBeNull();
});

it('reads the de bundle value under a de session', () => {
renderDrawerIn('de');

expect(screen.getByTitle('Schließen').getAttribute('aria-label')).toBe('Schließen');
expect(screen.getByRole('button', { name: 'Schließen' })).toBeTruthy();
const ours = screen.getByTitle('Schließen');
expect(ours.getAttribute('aria-label')).toBe('Schließen');
expect(screen.getAllByRole('button', { name: 'Schließen' })).toContain(ours);
expect(screen.queryByTitle('Close')).toBeNull();
});
});
Expand Down
123 changes: 123 additions & 0 deletions packages/components/src/__tests__/sheet-dialog-close-i18n.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* The Shadcn `Sheet`/`Dialog` close buttons speak the session locale —
* objectstack#5505.
*
* Both primitives auto-render a close button that is icon-only (a lucide `X`),
* so its `sr-only` span is not decoration: it IS the control's accessible
* name. Upstream ships that span as a hardcoded English literal, so under
* zh/ja/es every drawer and modal in the console — ~20 `SheetContent`
* consumers plus every `DialogContent` consumer (ChatDock, ActivityFeed,
* metadata-admin, AiChatPage, PeoplePicker, …) — announced "Close" in English.
*
* The span now renders `<CloseSrLabel />` (`../lib/close-label`), which
* resolves `common.close`. Because `src/ui/**` is regenerated from the
* registry, that one-line reference is DECLARED in
* `scripts/shadcn-local-patches.mjs` and re-applied by every sync; the patch
* mechanism itself is tested in
* `scripts/__tests__/shadcn-local-patches.test.ts`.
*
* ## Why the no-provider cases matter as much as the translated ones
*
* A large amount of existing coverage addresses these controls by their
* English name with NO `I18nProvider` mounted (`discardGuard.test.tsx`,
* `InlineCreateRelated.closeButtonName.test.tsx`, e2e specs). The safe
* translation's English fallback is what keeps those green, so it is pinned
* here explicitly rather than assumed.
*/

import { describe, it, expect, afterEach } from 'vitest';
import { render, screen, cleanup } from '@testing-library/react';
import { I18nProvider } from '@object-ui/i18n';
import { Sheet, SheetContent, SheetTitle, SheetDescription } from '../ui/sheet';
import { Dialog, DialogContent, DialogTitle, DialogDescription } from '../ui/dialog';

afterEach(() => cleanup());

/** Sheet/Dialog bodies carry a title + description so Radix has no a11y complaint. */
function sheetBody() {
return (
<Sheet open>
<SheetContent>
<SheetTitle>Acme Corp</SheetTitle>
<SheetDescription>Record drawer</SheetDescription>
</SheetContent>
</Sheet>
);
}

function dialogBody() {
return (
<Dialog open>
<DialogContent>
<DialogTitle>Acme Corp</DialogTitle>
<DialogDescription>Record modal</DialogDescription>
</DialogContent>
</Dialog>
);
}

function inLocale(language: string, body: React.ReactElement) {
return render(
<I18nProvider config={{ defaultLanguage: language, detectBrowserLanguage: false }}>
{body}
</I18nProvider>,
);
}

const primitives: Array<[string, () => React.ReactElement]> = [
['SheetContent', sheetBody],
['DialogContent', dialogBody],
];

describe.each(primitives)('%s close button — accessible name (objectstack#5505)', (_name, body) => {
it('reads English under an en session', () => {
inLocale('en', body());

expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy();
});

it('reads the zh bundle value under a zh session', () => {
inLocale('zh', body());

expect(screen.getByRole('button', { name: '关闭' })).toBeTruthy();
// The literal this replaced. Before the fix this query MATCHED under zh —
// that is the whole bug.
expect(screen.queryByRole('button', { name: 'Close' })).toBeNull();
expect(screen.queryByText('Close')).toBeNull();
});

it('reads the ja bundle value under a ja session', () => {
inLocale('ja', body());

expect(screen.getByRole('button', { name: '閉じる' })).toBeTruthy();
expect(screen.queryByRole('button', { name: 'Close' })).toBeNull();
});

it('reads the es bundle value under an es session', () => {
inLocale('es', body());

expect(screen.getByRole('button', { name: 'Cerrar' })).toBeTruthy();
expect(screen.queryByRole('button', { name: 'Close' })).toBeNull();
});
});

/**
* The English no-provider fallback is pinned in a FILE OF ITS OWN —
* `sheet-dialog-close-no-provider-fallback.test.tsx`.
*
* `createI18n` registers its instance as react-i18next's module-global
* default, and that registration survives unmount and `cleanup()`. So any
* "no provider" render placed in THIS file would silently resolve against
* whichever locale the tests above mounted last. Written here first, the
* fallback case failed with the button named "Cerrar" under a test that
* mounted no provider at all — see the same warning on
* `chrome-i18n-no-provider-fallback.test.tsx` (objectstack#5506).
*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* The `Sheet`/`Dialog` close buttons still resolve to ENGLISH when no
* `I18nProvider` is mounted — objectstack#5505.
*
* This is the load-bearing half of the fix, not a formality. Routing the
* primitives' `sr-only` label through `t()` touches every `SheetContent` and
* `DialogContent` consumer in the repo, and a large amount of existing
* coverage addresses those controls by their ENGLISH accessible name with no
* provider in the tree — `packages/plugin-form/src/discardGuard.test.tsx`
* (`getByRole('button', { name: 'Close' })`) and
* `packages/plugin-detail/src/__tests__/InlineCreateRelated.closeButtonName.test.tsx`
* (`/^Close$/`) among them. A `t()` call without a working default renders the
* raw `common.close` key and breaks all of it — in other packages' suites, not
* this one's.
*
* ── Why this is its own FILE, not a describe block ────────────────────────
* `createI18n` calls `instance.use(initReactI18next)`, which registers that
* instance as react-i18next's module-global default, and the registration
* survives unmount and `cleanup()`. The moment any test in a file mounts
* `<I18nProvider config={{ defaultLanguage: 'es' }}>`, every later "no
* provider" render in that same file resolves against the Spanish instance.
* Written as a describe block inside
* `sheet-dialog-close-i18n.test.tsx` this case failed with the button named
* "Cerrar" under a test that mounted no provider at all — the identical trap
* `chrome-i18n-no-provider-fallback.test.tsx` documents for objectstack#5506.
*
* Vitest's `dom` project runs with `isolate: true`, so a file that never
* mounts a provider gets a genuinely clean global. Keep it that way:
* **do not import or mount `I18nProvider` here.**
*/

import { describe, it, expect, afterEach } from 'vitest';
import { render, screen, cleanup } from '@testing-library/react';
import { Sheet, SheetContent, SheetTitle, SheetDescription } from '../ui/sheet';
import { Dialog, DialogContent, DialogTitle, DialogDescription } from '../ui/dialog';

afterEach(() => cleanup());

describe('Sheet/Dialog close — English fallback with no provider (objectstack#5505)', () => {
it('names SheetContent’s close button "Close"', () => {
render(
<Sheet open>
<SheetContent>
<SheetTitle>Acme Corp</SheetTitle>
<SheetDescription>Record drawer</SheetDescription>
</SheetContent>
</Sheet>,
);

expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy();
// Never the raw key — that is the shape that would break the consumers above.
expect(screen.queryByText('common.close')).toBeNull();
});

it('names DialogContent’s close button "Close"', () => {
render(
<Dialog open>
<DialogContent>
<DialogTitle>Acme Corp</DialogTitle>
<DialogDescription>Record modal</DialogDescription>
</DialogContent>
</Dialog>,
);

expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy();
expect(screen.queryByText('common.close')).toBeNull();
});
});
85 changes: 85 additions & 0 deletions packages/components/src/lib/close-label.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

"use client"

/**
* Translated accessible name for the close button the Shadcn `Sheet` and
* `Dialog` primitives auto-render — objectstack#5505.
*
* Those buttons are icon-only (a lucide `X`), so their `sr-only` span is not
* decoration: it IS the control's accessible name, the thing a screen reader
* announces and the thing `getByRole('button', { name })` matches. Upstream
* ships it as a hardcoded English literal, so under zh/ja/es every drawer and
* modal in the console announced "Close" in English.
*
* ## Why the implementation lives HERE and not in the primitives
*
* `packages/components/src/ui/**` is regenerated from the Shadcn registry
* (AGENTS.md Commandment #7) — anything written there is overwritten by the
* next `pnpm shadcn:update`. `src/lib/` is not part of that regeneration, so
* this file is permanent. The primitives get only a one-line
* `<CloseSrLabel />` reference, re-applied on every sync by the declared patch
* in `scripts/shadcn-local-patches.mjs`. Keeping the payload out of the
* regenerated zone is what makes the patch small enough to survive upstream
* churn: two anchored lines rather than an inlined hook.
*
* ## Why `createSafeTranslation`
*
* The no-provider path must stay English. A large number of existing unit
* tests and e2e specs address dialogs and drawers by their English accessible
* name with no `I18nProvider` mounted (e.g.
* `packages/plugin-form/src/discardGuard.test.tsx`,
* `packages/plugin-detail/src/__tests__/InlineCreateRelated.closeButtonName.test.tsx`),
* and a primitive that rendered a raw `common.close` key there would break all
* of them. `createSafeTranslation` probes its test key and falls back to the
* defaults map below when translations are not configured, so "no provider"
* resolves to "Close" rather than to a key.
*/

import * as React from "react"
import { createSafeTranslation } from "@object-ui/i18n"

/**
* The English fallback, used when no `I18nProvider` is mounted.
*
* `common.close` is deliberately the key the rest of the console already uses
* for a bare "Close" (present in all ten locale packs since objectstack#5430),
* not a new primitive-private one.
*
* Deliberately NOT exported: this file's only export is a component, which is
* what `react-refresh/only-export-components` wants, and nothing outside needs
* the map — consumers that want the string should read `common.close` from the
* locale packs like everyone else.
*/
const CLOSE_LABEL_DEFAULT_TRANSLATIONS: Record<string, string> = {
'common.close': 'Close',
}

/**
* Probe key is `common.close` itself: it exists in every shipped pack, so a
* mounted provider always resolves it and the real `t` is used; with no
* provider the probe fails and the default above is returned.
*/
const useCloseTranslation = createSafeTranslation(
CLOSE_LABEL_DEFAULT_TRANSLATIONS,
'common.close',
)

/**
* `sr-only` accessible name for an icon-only close control.
*
* A component rather than a helper call because it owns a hook — the
* primitives render it as `<CloseSrLabel />` inside their close button, which
* keeps the hook call legal in files whose components are expression-bodied
* `forwardRef` arrows with no statement block to put a hook in.
*/
export function CloseSrLabel(): React.ReactElement {
const { t } = useCloseTranslation()
return <span className="sr-only">{t('common.close')}</span>
}
3 changes: 2 additions & 1 deletion packages/components/src/ui/dialog.tsx

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading