diff --git a/.changeset/11811-disabled-action-reason.md b/.changeset/11811-disabled-action-reason.md new file mode 100644 index 0000000000..63e48b8ffd --- /dev/null +++ b/.changeset/11811-disabled-action-reason.md @@ -0,0 +1,22 @@ +--- +'@object-ui/i18n': minor +'@object-ui/components': minor +'@object-ui/app-shell': patch +'@object-ui/plugin-detail': patch +--- + +A record action greyed out by its declared `disabled` predicate now says why (objectui#11811). It shows the reason "Not available for this record", and the same text is its accessible description (`aria-describedby`), so a screen reader announces it too. Before, the action carried no tooltip, no `title` and no description, so a user could not learn why it was off. + +Where it shows: + +- **The record page header** (`page:header`, `@object-ui/components`). On an inline action button, hovering it or focusing it from the keyboard opens a tooltip with the reason. On an action in the ⋯ overflow menu, the reason is a second line under the label. A tooltip there could not be reached from the keyboard, because the menu skips a disabled item and traps Tab. +- **The `record:quick_actions` bar** (`@object-ui/plugin-detail`), for example a record page's section bar. Hovering or focusing the button opens the tooltip. +- **The `DeclaredActionsBar`** (`@object-ui/app-shell`), which renders server-declared actions on the approvals surfaces. Hovering or focusing the button opens the tooltip. + +A natively disabled button receives no pointer or focus events, so the tooltip's trigger is a focusable wrapper around the button, the pattern Radix documents for a disabled trigger. + +What stays unchanged: a button greyed out only while its own action runs shows no reason. So do the header's Edit and Delete that the console injects, whose `disabled` the host computes (for example while the record is locked for approval), and a header action greyed out by a live inline-edit session. + +The reason is the same generic sentence for every action. An author-written reason beside the predicate would be a new key on the action spec, which is objectstack's to declare. It is not part of this change. + +**Clause-②: yes (widening).** `@object-ui/i18n` gains one language-pack key, `actions.notAvailableForRecord`, translated in all ten packs. No export, prop or type member is added, removed or changed. diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index 6564ca0a4e..95b4cbbff9 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -31,7 +31,16 @@ */ import React, { useCallback, useMemo, useState } from 'react'; -import { Button, Separator, cn, hasDeclaredVisibilityGate } from '@object-ui/components'; +import { + Button, + Separator, + cn, + hasDeclaredVisibilityGate, + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from '@object-ui/components'; import { ActionProvider, useAction, @@ -178,6 +187,8 @@ const DeclaredActionButton: React.FC<{ // these two lines carried — which existed only because `ActionDef.disabled` // could not describe the envelope arm — have nothing left to reach around. const isDisabledPred = useCondition(toPredicateInput(action.disabled), predicateRecord); + // Called up here with the other hooks: the `visible` gate below returns early. + const reasonId = React.useId(); /** * Is the button this viewer is looking at an ADMIN OVERRIDE (objectui#5178)? @@ -371,28 +382,42 @@ const DeclaredActionButton: React.FC<{ // dialog body can never come from different bundle reads (objectui#4265). const label = isOverride ? overrideLabel : declaredLabel; - return ( + // Is a `disabled` gate DECLARED? The same question the `visible` gate + // above asks, so it reads the same definition rather than re-spelling it. + // The name is historic — objectui#3492 arrived through `visible` — and the + // predicate is key-neutral: "declared" is `!= null && !== ''`, because an + // empty predicate is nothing to evaluate. Kept under that name + // deliberately (objectui#3842 ruling): one implementation behind two names + // is a dialect, not a clarification. + // + // `!= null` alone was a real defect here, and NOT for the reason it was on + // `visible`: the evaluation entry reads an empty predicate as "no + // condition → true", which on `visible` means SHOW (so an over-broad + // "declared" test cancels out and `''` renders either way), but here means + // DISABLE. A `disabled: ''` on a server-declared approval action rendered + // a permanently greyed-out Approve / Reject — the mirror image of + // objectui#3835 on the same surface, and equally impossible to tell from + // deliberate metadata by looking at it. + // + // Held apart from `loading` (objectui#11811): only the declared predicate is + // a fact about the record, so only it earns the "not available" reason. A + // button greyed out while its own action runs already says why — the spinner. + const disabledByPredicate = hasDeclaredVisibilityGate(action.disabled) ? isDisabledPred : false; + // The generic reason (objectui#11811). An author-written reason beside the + // predicate would be a spec key, which is objectstack's to declare — not one + // this bar may invent — so every predicate-disabled action says the same + // thing for now. + const disabledReason = disabledByPredicate + ? String(t('actions.notAvailableForRecord', { defaultValue: 'Not available for this record' })) + : undefined; + + const button = ( ); + if (!disabledReason) return button; + // A natively `disabled` button fires no pointer or focus events, and the + // Button primitive adds `disabled:pointer-events-none` on top — so a + // tooltip (or a native `title`) on the button itself never opens. The + // wrapping span is the trigger instead, the idiom Radix documents for a + // disabled button: it takes the hover, and `tabIndex={0}` lets a keyboard + // user focus it, which opens the tooltip too. The reason is ALSO a + // persistent accessible description (`aria-describedby` on both the + // button and the span, onto an `sr-only` copy). Same shape as + // `record:quick_actions`' button and `DeclaredActionsBar`'s. + return ( + + + + + {button} + {disabledReason} + + + {disabledReason} + + + ); }; return (
= ({ schema, className, ...props }) => { const icon = typeof action.icon === 'string' ? action.icon : null; const isDestructive = action.variant === 'destructive' || action.name === 'sys_delete'; + // objectui#11811 — the same reason as the inline button, but + // drawn as a visible second line, not a tooltip. Inside the + // menu a tooltip trigger is unreachable from the keyboard: a + // disabled item is skipped by arrow-key focus (`focusable: + // !disabled` in the menu's roving group), and the menu traps + // Tab. The menu is already an explicit, opened surface, so the + // reason simply shows there, and is the item's description + // while its name stays the label alone. + const disabledReason = disabledReasonFor(action); + const labelId = `${disabledReasonIdBase}-more-label-${idx}`; + const reasonId = `${disabledReasonIdBase}-more-reason-${idx}`; return ( { e.preventDefault(); if (typeof action.onClick === 'function') { @@ -2139,7 +2218,14 @@ const PageHeaderRenderer: React.FC = ({ schema, className, ...props }) => { )} > {icon && } - {label} + {disabledReason ? ( + + {label} + {disabledReason} + + ) : ( + {label} + )} ); })} diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 412fb78213..22c75c613f 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -191,6 +191,7 @@ const ar = { copyAll: 'نسخ الكل', }, notAvailableHere: '"{{action}}" غير متاح في الصفحة الحالية.', + notAvailableForRecord: 'غير متاح لهذا السجل', completedSuccessfully: 'اكتمل الإجراء بنجاح', failed: 'فشل الإجراء', parallelFailed: 'فشل إجراء متوازٍ واحد أو أكثر', diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 1ba934913a..6dd6f05e70 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -155,6 +155,7 @@ const de = { copyAll: 'Alle kopieren', }, notAvailableHere: '„{{action}}“ ist auf der aktuellen Seite nicht verfügbar.', + notAvailableForRecord: 'Für diesen Datensatz nicht verfügbar', completedSuccessfully: 'Aktion erfolgreich abgeschlossen', failed: 'Aktion fehlgeschlagen', parallelFailed: 'Eine oder mehrere parallele Aktionen sind fehlgeschlagen', diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 31cb2707c2..77662ebdfc 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -184,6 +184,12 @@ const en = { // `visible` gate outranks (objectui#4191) — the deep link or host asked // for it, but the author hid it on this surface. notAvailableHere: '"{{action}}" is not available on the current page.', + // The reason a record action greyed out by its declared `disabled` + // predicate gives on hover, on focus and as its accessible description + // (objectui#11811): the generic one, since the action spec carries no + // author-written reason. Not `notAvailableHere` — that one is about the + // page, this one about the record. + notAvailableForRecord: 'Not available for this record', // The success toast the action runner shows when no `outcomeMessages` // entry applies to the answer and the action declares no `successMessage`; // the server's message plays no part (objectui#11344). It is the one diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index a906b00fe0..815839899e 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -160,6 +160,7 @@ const es = { copyAll: 'Copiar todo', }, notAvailableHere: '«{{action}}» no está disponible en la página actual.', + notAvailableForRecord: 'No disponible para este registro', completedSuccessfully: 'La acción se completó correctamente', failed: 'La acción falló', parallelFailed: 'Una o más acciones paralelas fallaron', diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index febbb84d33..c8ab7fbee4 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -161,6 +161,7 @@ const fr = { copyAll: 'Tout copier', }, notAvailableHere: '« {{action}} » n\'est pas disponible sur la page actuelle.', + notAvailableForRecord: 'Non disponible pour cet enregistrement', completedSuccessfully: 'Action effectuée avec succès', failed: "Échec de l'action", parallelFailed: 'Une ou plusieurs actions parallèles ont échoué', diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 7662fbd17d..df003e4b5d 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -155,6 +155,7 @@ const ja = { copyAll: 'すべてコピー', }, notAvailableHere: '「{{action}}」は現在のページでは利用できません。', + notAvailableForRecord: 'このレコードでは利用できません', completedSuccessfully: '操作が正常に完了しました', failed: '操作に失敗しました', parallelFailed: '1 つ以上の並列操作が失敗しました', diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 68215b223f..eff94c9cda 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -155,6 +155,7 @@ const ko = { copyAll: '모두 복사', }, notAvailableHere: '"{{action}}"은(는) 현재 페이지에서 사용할 수 없습니다.', + notAvailableForRecord: '이 레코드에서는 사용할 수 없습니다', completedSuccessfully: '작업이 완료되었습니다', failed: '작업 실패', parallelFailed: '하나 이상의 병렬 작업이 실패했습니다', diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 23987f28c9..7c39c12737 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -160,6 +160,7 @@ const pt = { copyAll: 'Copiar tudo', }, notAvailableHere: '"{{action}}" não está disponível na página atual.', + notAvailableForRecord: 'Não disponível para este registro', completedSuccessfully: 'A ação foi concluída com sucesso', failed: 'A ação falhou', parallelFailed: 'Uma ou mais ações paralelas falharam', diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 58ef4ed57b..3dcdcbd37f 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -175,6 +175,7 @@ const ru = { copyAll: 'Копировать всё', }, notAvailableHere: '«{{action}}» недоступно на текущей странице.', + notAvailableForRecord: 'Недоступно для этой записи', completedSuccessfully: 'Действие успешно выполнено', failed: 'Действие не выполнено', parallelFailed: 'Одно или несколько параллельных действий не выполнены', diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index be4f1d2bbd..cf107a629b 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -162,6 +162,7 @@ const zh = { copyAll: '全部复制', }, notAvailableHere: '「{{action}}」在当前页面不可用。', + notAvailableForRecord: '对此记录不可用', completedSuccessfully: '操作已成功完成', failed: '操作失败', parallelFailed: '一个或多个并行操作失败', diff --git a/packages/plugin-detail/src/renderers/__tests__/record-quick-actions.disabledReason-11811.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-quick-actions.disabledReason-11811.test.tsx new file mode 100644 index 0000000000..0e542187a6 --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-quick-actions.disabledReason-11811.test.tsx @@ -0,0 +1,151 @@ +/** + * 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. + */ + +/** + * objectui#11811 — a `record:quick_actions` button greyed out by its declared + * `disabled` predicate says why. + * + * The card's reproduction is the showcase Task page: its section bar is this + * block, and *Archive* declares `disabled: 'has(record.done) && record.done + * != true'`. Before the fix the greyed-out button carried no tooltip, no + * `title` and no `aria-describedby`, and a natively disabled button (with the + * Button primitive's `disabled:pointer-events-none`) never fires the hover or + * focus a tooltip would need. The reason now rides a focusable wrapper span — + * the tooltip trigger — and a persistent `sr-only` description both the + * button and the span point at. + * + * Nothing is stubbed: the predicate runs through the real `useCondition`, the + * tooltip is the real Radix one from `@object-ui/components`, and the zh case + * reads the real zh pack through a real `I18nProvider`. + */ + +import * as React from 'react'; +import { describe, it, expect } from 'vitest'; +import { render, screen, within, act, fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import '@testing-library/jest-dom'; +import { RecordContextProvider } from '@object-ui/react'; +import { I18nProvider } from '@object-ui/i18n'; +import { RecordQuickActionsRenderer } from '../record-quick-actions'; + +const REASON_EN = 'Not available for this record'; +const REASON_ZH = '对此记录不可用'; + +/** + * The showcase specimen, in the shape the server SERVES it: the authored CEL + * string compiled into a `{ dialect: 'cel', source }` envelope. The bare + * string would take the legacy `${…}` evaluator, where `has()` faults — and a + * faulting `disabled` greys the action out on both rows (fail-soft), so the + * "disabled" case would pass for the wrong reason and the control below would + * go red. The envelope reaches a real verdict both ways. + */ +const ARCHIVE = { + name: 'showcase_archive_task', + label: 'Archive', + type: 'script', + locations: ['record_section'], + disabled: { dialect: 'cel', source: 'has(record.done) && record.done != true' }, +}; + +function mount( + action: Record, + record: Record, + wrap: (node: React.ReactElement) => React.ReactElement = (node) => node, +) { + return render( + wrap( + + + , + ), + ); +} + +const archive = () => screen.getByRole('button', { name: 'Archive' }); +/** The element the button's `aria-describedby` names, or `null`. */ +const describedBy = (el: HTMLElement) => { + const id = el.getAttribute('aria-describedby'); + return id ? document.getElementById(id) : null; +}; + +describe('record:quick_actions — a predicate-disabled action says why (objectui#11811)', () => { + it('disabled by its predicate: the button carries the reason as its accessible description', () => { + mount(ARCHIVE, { done: false }); + const button = archive(); + expect(button).toBeDisabled(); + expect(describedBy(button)).toHaveTextContent(REASON_EN); + // The reason is a DESCRIPTION, never folded into the accessible name. + expect(button).toHaveAccessibleName('Archive'); + }); + + it('hovering the disabled action opens a tooltip with the reason', async () => { + const user = userEvent.setup(); + mount(ARCHIVE, { done: false }); + const trigger = archive().parentElement as HTMLElement; + expect(trigger).toHaveAttribute('data-disabled-reason'); + await user.hover(trigger); + const tooltip = await screen.findByRole('tooltip'); + expect(tooltip).toHaveTextContent(REASON_EN); + }); + + it('a keyboard user reaches the reason: Tab lands on the trigger, which opens the tooltip and is described by it', async () => { + const user = userEvent.setup(); + mount(ARCHIVE, { done: false }); + const trigger = archive().parentElement as HTMLElement; + await user.tab(); + expect(trigger).toHaveFocus(); + expect(describedBy(trigger)).toHaveTextContent(REASON_EN); + const tooltip = await screen.findByRole('tooltip'); + expect(tooltip).toHaveTextContent(REASON_EN); + }); + + it('control — the predicate does not hold: the action is live and says nothing', () => { + mount(ARCHIVE, { done: true }); + const button = archive(); + expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute('aria-describedby'); + expect(button.parentElement).not.toHaveAttribute('data-disabled-reason'); + expect(screen.queryByText(REASON_EN)).toBeNull(); + }); + + it('control — a button greyed out only while its own action runs gives no "not available" reason', async () => { + let finish: () => void = () => {}; + const pending = new Promise((resolve) => { finish = resolve; }); + // No `disabled` key: the running state is the ONLY reason it greys out. + const SLOW = { name: 'slow', label: 'Slow', type: 'script', locations: ['record_section'], onClick: () => pending }; + mount(SLOW, {}); + const button = screen.getByRole('button', { name: 'Slow' }); + fireEvent.click(button); + expect(screen.getByRole('button', { name: 'Slow' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Slow' })).not.toHaveAttribute('aria-describedby'); + expect(screen.queryByText(REASON_EN)).toBeNull(); + await act(async () => { finish(); await pending; }); + expect(screen.getByRole('button', { name: 'Slow' })).not.toBeDisabled(); + }); + + it('the reason comes from the language pack — zh', async () => { + mount(ARCHIVE, { done: false }, (node) => ( + + {node} + + )); + const reason = await screen.findByText(REASON_ZH); + expect(describedBy(archive())).toBe(reason); + expect(within(archive().parentElement as HTMLElement).queryByText(REASON_EN)).toBeNull(); + }); + + it('the reason comes from the language pack — en', async () => { + mount(ARCHIVE, { done: false }, (node) => ( + + {node} + + )); + const reason = await screen.findByText(REASON_EN); + expect(describedBy(archive())).toBe(reason); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-quick-actions.tsx b/packages/plugin-detail/src/renderers/record-quick-actions.tsx index eff51b7b6c..59c98158e0 100644 --- a/packages/plugin-detail/src/renderers/record-quick-actions.tsx +++ b/packages/plugin-detail/src/renderers/record-quick-actions.tsx @@ -18,7 +18,16 @@ import React from 'react'; import { useRecordContext, useActionEngine, useMetadataItem, useCondition, toPredicateInput, useActionTextLocalizer } from '@object-ui/react'; import { usePermissions } from '@object-ui/permissions'; -import { Button, cn, hasDeclaredVisibilityGate } from '@object-ui/components'; +import { + Button, + cn, + hasDeclaredVisibilityGate, + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from '@object-ui/components'; +import { useSafeTranslate } from '@object-ui/i18n'; import { Loader2 } from 'lucide-react'; import type { ActionDef, ActionLocation } from '@object-ui/core'; import { resolveDeclaredActionIds } from '@object-ui/types'; @@ -360,6 +369,8 @@ function QuickActionButton({ // engine. The previous `typeof === 'string'` split dropped the envelope, so // a spec-authored `disabled` never disabled anything on this surface. const isDisabledPred = useCondition(toPredicateInput((action as any).disabled), recordCtx); + const tt = useSafeTranslate(); + const reasonId = React.useId(); // Is a `disabled` gate DECLARED? Read from the one definition on the action // face rather than re-spelled here (objectui#3842 ruling, applied to this // site by #3849 — the historic `visible`-flavoured name is kept on purpose; @@ -370,12 +381,24 @@ function QuickActionButton({ // `disabled` means DISABLE — so `disabled: ''` (an empty predicate, i.e. // nothing declared) greyed this quick action out permanently, with no way for // the author to un-grey it. There is no legacy `enabled` leg on this surface. - const isDisabled = (hasDeclaredVisibilityGate((action as any).disabled) ? isDisabledPred : false) || running; - return ( + // + // Kept apart from `running` (objectui#11811): only the DECLARED predicate is + // a fact about the record, so only it earns the "not available" reason. A + // button greyed out while its own action runs already says why — the spinner. + const disabledByPredicate = hasDeclaredVisibilityGate((action as any).disabled) ? isDisabledPred : false; + const isDisabled = disabledByPredicate || running; + // The generic reason (objectui#11811). The author-written reason beside the + // predicate is a spec question for objectstack, not a key this renderer may + // invent, so every predicate-disabled action says the same thing for now. + const disabledReason = disabledByPredicate + ? tt('actions.notAvailableForRecord', 'Not available for this record') + : undefined; + const button = ( ); + if (!disabledReason) return button; + // A natively `disabled` button fires no pointer or focus events, and the + // Button primitive adds `disabled:pointer-events-none` on top — so a tooltip + // (or a native `title`) on the button itself never opens: the card's "hovering + // or focusing it shows nothing". The wrapping span is the trigger instead, + // the idiom Radix documents for a disabled button: it takes the hover, and + // `tabIndex={0}` lets a keyboard user focus it, which opens the tooltip too. + // The reason is ALSO a persistent accessible description (`aria-describedby` + // on both the button and the span, onto an `sr-only` copy), so a screen + // reader reaching either one hears it without the tooltip being open. Same + // shape as `DeclaredActionsBar`'s button in `@object-ui/app-shell`. + return ( + + + + + {button} + {disabledReason} + + + {disabledReason} + + + ); } export default RecordQuickActionsRenderer;