diff --git a/.changeset/12883-kanban-card-enter.md b/.changeset/12883-kanban-card-enter.md new file mode 100644 index 0000000000..0306ee01c8 --- /dev/null +++ b/.changeset/12883-kanban-card-enter.md @@ -0,0 +1,10 @@ +--- +'@object-ui/plugin-kanban': patch +--- + +A focused kanban card opens on Enter and on Space (objectui#12883). Every card was already a keyboard tab stop, but its only activation was the pointer click, so a keyboard user could reach a card and could not open its record from the board. + +- Enter, and Space on its release, call the same `onCardClick` a pointer click calls, handing it the key event. Its readers take `target` (the popover anchor) and `metaKey` / `ctrlKey` (open in a new tab) from it, which a key event carries; a key event has no `button`, so it never reads as a middle click. +- The card is now a `button` inside a `listitem`, the shape for an activatable item in a list. The list item keeps its name, and the card's description (objectui#12837) stays on the focusable button. +- A key pressed on an element inside a card, such as a link a field renders, does not open the card. +- Keyboard drag is unchanged: the board still builds no keyboard sensor. diff --git a/examples/schema-catalog/test/kanban-column-cards-6939.test.tsx b/examples/schema-catalog/test/kanban-column-cards-6939.test.tsx index 53a8188f01..583ba8238a 100644 --- a/examples/schema-catalog/test/kanban-column-cards-6939.test.tsx +++ b/examples/schema-catalog/test/kanban-column-cards-6939.test.tsx @@ -221,12 +221,16 @@ const DND_KIT_DEFAULT_INSTRUCTIONS = const KANBAN_DND_INSTRUCTIONS_EN = 'To move this card to another column, drag it with a pointer or by touch.'; /** - * The AUTHORED (`cards`) spelling as it reads TODAY, after objectui#12837 - * replaced the board's instructions text. Written as its own literal, not - * derived from `PRE_REPAIR_PRE_12837`, so the delta case compares two - * independently written readings. + * The AUTHORED (`cards`) spelling after objectui#12837 replaced the board's + * instructions text. Written as its own literal, not derived from + * `PRE_REPAIR_PRE_12837`, so the delta case compares two independently + * written readings. + * + * ⚠️ SUPERSEDED as the expected reading by objectui#12883 (see `PRE_REPAIR` + * below), and kept: the objectui#12837 delta case ends on it, and the + * objectui#12883 delta case starts from it. */ -const PRE_REPAIR: Record<(typeof IDS)[number], Reading> = { +const PRE_REPAIR_PRE_12883: Record<(typeof IDS)[number], Reading> = { 'plugin-kanban/basic-kanban-board': { elements: 64, tags: { DIV: 52, SPAN: 6, H3: 3, STYLE: 3 }, @@ -241,6 +245,29 @@ const PRE_REPAIR: Record<(typeof IDS)[number], Reading> = { }, }; +/** + * The AUTHORED (`cards`) spelling as it reads TODAY, after objectui#12883 made + * each card a `button` inside its own `listitem` wrapper: one more `
` per + * card, and nothing else — no text moved. Written as its own literal, not + * derived from `PRE_REPAIR_PRE_12883`, so the delta case below compares two + * independently written readings. The `items` readings do not move: that + * board draws no card. + */ +const PRE_REPAIR: Record<(typeof IDS)[number], Reading> = { + 'plugin-kanban/basic-kanban-board': { + elements: 68, + tags: { DIV: 56, SPAN: 6, H3: 3, STYLE: 3 }, + sha256: 'fe90e1cc6d10ddd0ba415df89ecdf6be451a9ba1831ce4adfdacbbb77702d263', + visibleText: "To Do2Design new featureCreate mockups and wireframesWrite documentationUpdate API docsIn Progress1Implement featureCode the new componentHigh PriorityDone1Setup projectInitialize repositoryTo move this card to another column, drag it with a pointer or by touch.", + }, + 'plugin-kanban/advanced-kanban-with-badges-and-limits': { + elements: 90, + tags: { DIV: 72, SPAN: 10, H3: 4, STYLE: 4 }, + sha256: 'f80e461758a7005a05d13bd53afe4604cc9b465cbb9d6e70961108b69fdec303', + visibleText: "Backlog2User AuthenticationImplement OAuth2.0BackendHighDark ModeAdd theme switcherUIWork In Progress1 / 3API IntegrationConnect to backend servicesIn ProgressCode Review0No cardsCompleted1Project SetupRepository initializedTo move this card to another column, drag it with a pointer or by touch.", + }, +}; + /** * The `items` spelling as it reads TODAY, after objectui#12837. objectui#6939's * result is unchanged: the board is still empty, says "No cards", and reads @@ -480,7 +507,7 @@ describe('objectui#6939 — and the repair moved the validator, not the renderer // instructions node every card's `aria-describedby` points at stays, and // only its text changes from dnd-kit's default to the bundle sentence. const pairs = [ - ['authored (`cards`)', PRE_REPAIR_PRE_12837[id], PRE_REPAIR[id]], + ['authored (`cards`)', PRE_REPAIR_PRE_12837[id], PRE_REPAIR_PRE_12883[id]], ['`items`', ITEMS_SPELLING_PRE_12837[id], ITEMS_SPELLING[id]], ] as const; for (const [spelling, before, after] of pairs) { @@ -499,4 +526,31 @@ describe('objectui#6939 — and the repair moved the validator, not the renderer expect(after.sha256, `${spelling}: one hash referenced twice`).not.toBe(before.sha256); } }); + + it.each(IDS)('%s: the authored census moved by exactly one wrapper
per card (objectui#12883)', (id) => { + // The same discipline again. objectui#12883 made each card a `button` (the + // control Enter and Space activate) inside a `listitem` wrapper, because a + // `button` cannot itself be the item its `role="list"` needs. That adds + // one `
` per card and nothing else. The card count is derived from + // the DOCUMENT, so a fixture that gains a card cannot satisfy this with + // the other entry's number. + const before = PRE_REPAIR_PRE_12883[id]; + const after = PRE_REPAIR[id]; + const cardCount = (getExample(id).schema as { columns: Array<{ cards: unknown[] }> }).columns.reduce( + (n, c) => n + c.cards.length, + 0, + ); + expect(cardCount, 'a board with no card would make this delta vacuous').toBeGreaterThan(0); + + // (1) exactly one element per card, and every one of them is a
. + expect(after.elements).toBe(before.elements + cardCount); + const { DIV: divBefore, ...otherBefore } = before.tags; + const { DIV: divAfter, ...otherAfter } = after.tags; + expect(divAfter, 'not one
per card').toBe(divBefore + cardCount); + expect(otherAfter, 'a tag other than
moved').toEqual(otherBefore); + + // (2) ⛔ no text moved: the wrapper draws nothing. + expect(after.visibleText, 'the visible text moved').toBe(before.visibleText); + expect(after.sha256, 'the text hash moved').toBe(before.sha256); + }); }); diff --git a/packages/plugin-kanban/src/KanbanImpl.tsx b/packages/plugin-kanban/src/KanbanImpl.tsx index 4570766afa..e5e8881574 100644 --- a/packages/plugin-kanban/src/KanbanImpl.tsx +++ b/packages/plugin-kanban/src/KanbanImpl.tsx @@ -153,10 +153,19 @@ export type { KanbanCard, KanbanColumn } from './types' // `@object-ui/types` gives the grid's and list view's rule (objectui#6349, // batch 7); no package entry reached the alias. +/** + * The event a card activation hands `onCardClick`: the pointer click, or the + * Enter / Space key that activates a focused card the same way + * (objectui#12883). Both carry what the readers of this channel take from it — + * `target` for the popover anchor, `metaKey` / `ctrlKey` for open-in-new-tab; + * a key event has no `button`, so it never reads as a middle click. + */ +export type KanbanCardActivationEvent = React.MouseEvent | React.KeyboardEvent + export interface KanbanBoardProps { columns: KanbanColumn[] onCardMove?: (cardId: string, fromColumnId: string, toColumnId: string, newIndex: number) => void - onCardClick?: (card: KanbanCard, event?: React.MouseEvent) => void + onCardClick?: (card: KanbanCard, event?: KanbanCardActivationEvent) => void className?: string quickAdd?: boolean onQuickAdd?: (columnId: string, title: string) => void @@ -218,7 +227,7 @@ function getCardStyles( return resolveConditionalFormatting(card as Record, rules as any, scope, objectFields as never) as React.CSSProperties } -function SortableCard({ card, onCardClick, conditionalFormatting, objectFields }: { card: KanbanCard; onCardClick?: (card: KanbanCard, event?: React.MouseEvent) => void; conditionalFormatting?: KanbanConditionalFormattingRule[]; objectFields?: unknown }) { +function SortableCard({ card, onCardClick, conditionalFormatting, objectFields }: { card: KanbanCard; onCardClick?: (card: KanbanCard, event?: KanbanCardActivationEvent) => void; conditionalFormatting?: KanbanConditionalFormattingRule[]; objectFields?: unknown }) { // A disabled sortable attaches no drag listeners and reports // `aria-disabled` (objectui#12082). const movable = React.useContext(KanbanCardsMovable) @@ -244,8 +253,38 @@ function SortableCard({ card, onCardClick, conditionalFormatting, objectFields } const owner = (card as { cardOwner?: KanbanCardOwner }).cardOwner return ( -
+
onCardClick?.(card, e)} + onKeyDown={(e) => { + // A key on something inside the card (a link a field renders) is that + // element's own; only the card's own key opens it. + if (e.target !== e.currentTarget) return + if (e.key === 'Enter') { + e.preventDefault() + onCardClick?.(card, e) + } else if (e.key === ' ') { + // A button activates on Space's release; held here, the press + // would scroll the board. No sensor binds a key (the board builds + // pointer and touch only), so Space is free. + e.preventDefault() + } + }} + onKeyUp={(e) => { + if (e.target !== e.currentTarget) return + if (e.key === ' ') { + e.preventDefault() + onCardClick?.(card, e) + } + }} > {card.coverImage && ( @@ -328,6 +367,7 @@ function SortableCard({ card, onCardClick, conditionalFormatting, objectFields } )}
+
) } @@ -607,7 +647,7 @@ function KanbanColumnView({ }: { column: KanbanColumn cards: KanbanCard[] - onCardClick?: (card: KanbanCard, event?: React.MouseEvent) => void + onCardClick?: (card: KanbanCard, event?: KanbanCardActivationEvent) => void quickAdd?: boolean onQuickAdd?: (columnId: string, title: string) => void conditionalFormatting?: KanbanConditionalFormattingRule[] diff --git a/packages/plugin-kanban/src/ObjectKanban.moveGrant-12082.test.tsx b/packages/plugin-kanban/src/ObjectKanban.moveGrant-12082.test.tsx index c6ec82da19..7cdd86a896 100644 --- a/packages/plugin-kanban/src/ObjectKanban.moveGrant-12082.test.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.moveGrant-12082.test.tsx @@ -120,7 +120,7 @@ async function mountBoard(perms: MePermissionsResponse | null) { } /** The card's own element — the sortable node, whose `aria-disabled` dnd-kit sets. */ -const cardNode = () => screen.getByRole('listitem', { name: CARD }); +const cardNode = () => screen.getByRole('button', { name: CARD }); async function dropOnInProgress() { expect(dnd.onDragEnd).toBeTypeOf('function'); diff --git a/packages/plugin-kanban/src/__tests__/cardKeyboardActivation-12883.test.tsx b/packages/plugin-kanban/src/__tests__/cardKeyboardActivation-12883.test.tsx new file mode 100644 index 0000000000..d6eb20c3d7 --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/cardKeyboardActivation-12883.test.tsx @@ -0,0 +1,146 @@ +/** + * 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. + */ + +/** + * A focused kanban card opens on Enter and on Space, through the SAME + * `onCardClick` a pointer click takes — objectui#12883. + * + * The defect: every card is a keyboard tab stop (dnd-kit's `tabIndex`), but + * its only activation was `onClick`, so Enter and Space on a focused card did + * nothing and a keyboard user could not open a record from the board. The + * card was a bare `listitem`, a role no key activates. + * + * The instrument: the real board, the real `DndContext`, real DOM events. + * Space is free for the card: the board builds no `KeyboardSensor` + * (objectui#12837's pin), and only that sensor binds a key. + * + * The controls: a key on an element inside the card is that element's own and + * opens nothing; a pointer click still calls `onCardClick` once, with the click. + */ + +import '@testing-library/jest-dom/vitest' +import * as React from 'react' +import { afterEach, describe, expect, it } from 'vitest' +import { cleanup, fireEvent, render, screen, within } from '@testing-library/react' +import { I18nProvider } from '@object-ui/i18n' +import type { KanbanCard, KanbanColumn } from '../types' +import KanbanBoard from '../KanbanImpl' + +const TITLE = 'Content backlog' + +const columns = (): KanbanColumn[] => [ + { + id: 'todo', + title: 'To Do', + cards: [ + { + id: 'c1', + title: TITLE, + // Two interactive elements inside the card: the controls' targets. + cardFieldCells: [ + { field: 'site', label: 'Site', node: example.com }, + { field: 'note', label: 'Note', node: }, + ], + }, + ], + }, + { id: 'done', title: 'Done', cards: [] }, +] + +type Call = { card: KanbanCard; type: string | undefined } + +async function renderBoard() { + const calls: Call[] = [] + render( + + {}} + onCardClick={(card, event) => calls.push({ card, type: event?.type })} + /> + , + ) + await screen.findByText('No cards') + return calls +} + +/** The card's focusable control. */ +const card = () => screen.getByRole('button', { name: TITLE }) + +afterEach(() => { + cleanup() +}) + +describe('a focused kanban card opens on Enter and Space, as a click does (objectui#12883)', () => { + it('the card is a button inside the listitem its list needs', async () => { + await renderBoard() + const item = screen.getByRole('listitem', { name: TITLE }) + expect(within(item).getByRole('button', { name: TITLE })).toBe(card()) + expect(item.closest('[role="list"]')).not.toBeNull() + // Still a tab stop: the focus is what the keys act on. + expect(card()).toHaveAttribute('tabindex', '0') + }) + + it('Enter on the focused card calls onCardClick once, with that card and the key event', async () => { + const calls = await renderBoard() + const el = card() + el.focus() + expect(el).toHaveFocus() + fireEvent.keyDown(el, { key: 'Enter', code: 'Enter' }) + expect(calls).toHaveLength(1) + expect(calls[0].card.id).toBe('c1') + expect(calls[0].type).toBe('keydown') + }) + + it('Space activates on release, as a button does; the press itself opens nothing', async () => { + const calls = await renderBoard() + const el = card() + el.focus() + const pressed = fireEvent.keyDown(el, { key: ' ', code: 'Space' }) + // The press is claimed (no page scroll) and opens nothing yet. + expect(pressed).toBe(false) + expect(calls).toHaveLength(0) + fireEvent.keyUp(el, { key: ' ', code: 'Space' }) + expect(calls).toHaveLength(1) + expect(calls[0].card.id).toBe('c1') + expect(calls[0].type).toBe('keyup') + }) + + it('CONTROL: a key on an element inside the card does not open the card', async () => { + const calls = await renderBoard() + const link = screen.getByRole('link', { name: 'example.com' }) + const input = screen.getByRole('textbox', { name: 'Note' }) + expect(card()).toContainElement(link) + expect(card()).toContainElement(input) + for (const el of [link, input]) { + fireEvent.keyDown(el, { key: 'Enter', code: 'Enter' }) + fireEvent.keyDown(el, { key: ' ', code: 'Space' }) + fireEvent.keyUp(el, { key: ' ', code: 'Space' }) + } + expect(calls).toHaveLength(0) + }) + + it('CONTROL: other keys on the focused card open nothing', async () => { + const calls = await renderBoard() + const el = card() + el.focus() + for (const key of ['a', 'Tab', 'Escape', 'ArrowDown']) { + fireEvent.keyDown(el, { key }) + fireEvent.keyUp(el, { key }) + } + expect(calls).toHaveLength(0) + }) + + it('CONTROL: a pointer click still calls onCardClick once, with the click', async () => { + const calls = await renderBoard() + fireEvent.click(card()) + expect(calls).toHaveLength(1) + expect(calls[0].card.id).toBe('c1') + expect(calls[0].type).toBe('click') + }) +}) diff --git a/packages/plugin-kanban/src/__tests__/dndAnnouncements-12716.test.tsx b/packages/plugin-kanban/src/__tests__/dndAnnouncements-12716.test.tsx index 9142eb2f4f..c395e9c442 100644 --- a/packages/plugin-kanban/src/__tests__/dndAnnouncements-12716.test.tsx +++ b/packages/plugin-kanban/src/__tests__/dndAnnouncements-12716.test.tsx @@ -99,7 +99,8 @@ async function renderBoard(lang: 'en' | 'zh') { /** What the live region says now. */ const said = () => document.querySelector('[id^="DndLiveRegion"]')?.textContent ?? null -const card = (title: string) => screen.getByRole('listitem', { name: title }) +// The sortable node — the `button` inside each card's `listitem` (objectui#12883). +const card = (title: string) => screen.getByRole('button', { name: title }) /** * Pick the card up with the pointer, then move it through `bands` (band 1, 2, @@ -217,7 +218,7 @@ describe('the kanban board announces titles and labels, not ids (objectui#12716) , ) await screen.findByText(NO_CARDS.en) - const el = document.querySelector('[aria-roledescription="sortable"][role="listitem"]') as HTMLElement + const el = document.querySelector('[aria-roledescription="sortable"][role="button"]') as HTMLElement el.dataset.cardId = CONTENT const heard = pointerDrag(el, [DOING]) expect(heard[0]).toBe('Picked up an untitled card in To Do.') diff --git a/packages/plugin-kanban/src/__tests__/dndInstructions-12837.test.tsx b/packages/plugin-kanban/src/__tests__/dndInstructions-12837.test.tsx index d2f4e57864..c7874047af 100644 --- a/packages/plugin-kanban/src/__tests__/dndInstructions-12837.test.tsx +++ b/packages/plugin-kanban/src/__tests__/dndInstructions-12837.test.tsx @@ -72,7 +72,9 @@ async function renderBoard(lang: 'en' | 'zh') { await screen.findByText(NO_CARDS[lang]) } -const card = () => screen.getByRole('listitem', { name: 'Content backlog' }) +// The focusable control, which carries the description; since objectui#12883 +// it is the `button` inside the card's `listitem`. +const card = () => screen.getByRole('button', { name: 'Content backlog' }) /** The text of the node the card's `aria-describedby` points at. */ function description(el: HTMLElement): string {