Repository navigation
Commit 33e3583
fix(app-shell): the form designer's drag live region names labels and places, not ids (objectui#11802) (#11869)
Fixes #11802
Clause-②: no
## What changed
The Studio form designer's `DndContext` passed no `accessibility` prop,
so dnd-kit read its default English sentences to a screen reader, naming
the canvas ids. A real Chromium reading of the live region before this
change, dragging *Name* into *New group*:
```text
Picked up draggable item f:name.
Draggable item f:name was moved over droppable area f:name.
Draggable item f:name was moved over droppable area g:__ungrouped__.
Draggable item f:name is no longer over a droppable area.
Draggable item f:name was moved over droppable area g:new_group.
Draggable item f:name was moved over droppable area f:name.
Draggable item f:name was dropped over droppable area f:name
```
The `DndContext` now takes `accessibility={dndAccessibility}`:
- **Every sentence is localized, en and zh,** under
`engine.studio.formDnd.*` in the designer's own table: pick-up
(`onDragStart`), over (`onDragOver`, with a not-over-a-group form), drop
(`onDragEnd`, with a dropped-outside form), cancel (`onDragCancel`), and
the screen-reader instructions dnd-kit points every card's
`aria-describedby` at.
- **Each names labels, never an id.** The field label comes from
`fieldLabelOf` (the resolver the cards render), the group label from
`labelOf` (the resolver the section headers render). No second map is
built.
- **The place is "N of M" inside the target group.** It is read off the
container map `onDragEnd` reads, with `onDragEnd`'s own arithmetic
(`dropPlaceIn`, beside the id helpers), so the place a drop announces is
the place it commits. A drop outside every group, or a cancel, names the
place the field is back in (the draft's layout, which the handlers
restore).
- **A sentence about the place the region already named is skipped.**
dnd-kit replaces the region's text on every event, and a pointer drag's
first `onDragOver` is the field over its own card; before this change
that overwrote the pick-up sentence before the browser's next observer
callback.
The wording lives in a new module-private helper,
`formDndAnnouncements.ts` (`formDndAccessibility(locale, lookups)`),
because `ObjectFormDesigner.tsx` exports components only.
**Unchanged:** the drag handlers (`onDragStart` / `onDragOver` /
`onDragEnd` / `onDragCancel`), the layout model, `isKeptOffLayout`, the
sensors and the collision detection. No export, prop, `@object-ui/types`
member, accepted input, `package.json` or `packages/i18n` key is added.
The designer hint and the Form tab caption are not touched.
## The same drag after this change (real Chromium, `8f5fb15`)
```text
Picked up Name. It is in Ungrouped, position 1 of 2.
Name is over Ungrouped, position 2 of 2.
Name is not over a group.
Name is over New group, position 1 of 1.
Name moved to New group, position 1 of 1.
```
zh, with the object translations the canvas shows:
```text
已拿起 名称,当前在 未分组,第 1 个,共 2 个。
名称 正移到 未分组,第 2 个,共 2 个。
名称 不在任何分组上。
名称 正移到 新分组,第 1 个,共 1 个。
名称 已移到 新分组,第 1 个,共 1 个。
```
The committed draft put `name` in `new_group` both times. A second drag
(*Industry* carried across *New group* onto *Phone* in *Contact*) ended
"Industry moved to Contact, position 1 of 2." and the draft had
`industry` first in `contact`. The readings came from a dev-only harness
that mounts `ObjectFormDesigner` with an `I18nProvider` in the console's
Vite dev server; a MutationObserver with `characterDataOldValue`
recorded every value the region took, including ones replaced before the
observer ran. The harness was never committed.
## The PM's hypotheses, measured
- **H1, confirmed.** One `DndContext` in the designer; each section's
`SortableContext` sits inside it, and no other dnd context exists in
`packages/app-shell/src`. Fields are sortables `f:` + field name
(draggable and droppable); sections are droppables `g:` + group key, the
ungrouped bucket included (`g:__ungrouped__`). Sections are not
draggable; they move by buttons.
- **H2, confirmed.** `entryByName` + `fieldLabelOf` and `labelOf` are
the lookups the cards and section headers render from. The announcements
reuse them.
- **H3, corrected.** dnd-kit's `LiveRegion` is `role="status"`,
`aria-live="assertive"`, `aria-atomic="true"`, and it is not debounced:
`announce` sets the text at once, and each sentence replaces the one
before. Measured on the drag above: before, 7 texts, two of them
replaced before the observer's next callback ran (the pick-up sentence
among them); after, 5 texts, each at least about 100 ms after the
previous one. A longer drag across two groups still gives one sentence
per change of what the pointer is over, a few of them about 20 ms apart
while the cards shift under the pointer. Each such sentence names a
place, so it is kept: an over sentence with the place is what a keyboard
user needs on each arrow key, and only the repeat of a place already
spoken is skipped.
## Tests
New: `ObjectFormDesigner.dndAnnouncements-11802.test.tsx`, 8 cases,
reading the real live region and the real instructions. The real
`DndContext`, sensors and handlers run; only `collisionDetection` is
replaced by a test geometry where the pointer's x picks the droppable,
because the test DOM measures every box as zero.
- Name into New group: pick-up, over and drop, each with labels and
place; "Name moved to New group, position 1 of 1." No sentence matches
`\b[fg]:\w`.
- The same in zh: "名称 已移到 新分组,第 1 个,共 1 个。"
- The localized instructions every card's `aria-describedby` points at,
en and zh.
- A reorder inside one group; a drop on a field of another group after
the canvas moved the field; a drop still over the field it was carried
onto (the handler puts it after that field).
- Leaving every group then dropping: "Name is not over a group." and the
dropped-outside sentence; nothing committed.
- Keyboard pick-up and Escape cancel.
**Control:** every drop case compares the announced place with the place
the committed `fields` give the field. The existing move test,
`DataPillar.hiddenSystemFields-11780.test.tsx` (its write-back drop),
runs unchanged and green.
**Reverse check** (fix committed first; `ObjectFormDesigner.tsx` checked
out from the base `5aa7f55`; `accessibility={dndAccessibility}` count 1,
then 0 on disk; restored with `git checkout HEAD --`; restored blob
equal to the HEAD blob, `git diff HEAD` and `git diff --cached` empty):
8 of 8 failed in the predicted direction, the live region carrying the
ids. The first: expected "Picked up Name. It is in Ungrouped, position 1
of 2.", received "Picked up draggable item f:name.".
## Gates (run on `8f5fb15`)
| Command | Exit | Verdict |
|---|---|---|
| `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...'
build` | 0 | 28 buildable packages Done (29 in scope;
`@object-ui/test-support` has no build script) |
| `pnpm --filter @object-ui/app-shell type-check` | 0 | both `tsc`
passes; `tsconfig.test.json --listFiles` includes the new test |
| `pnpm exec vitest run` over the 8 files below | 0 | 8 files, 247 tests
passed |
| `pnpm exec eslint --no-inline-config` on the 4 touched source files |
0 | 0 errors; 1 warning, `react-hooks/set-state-in-effect`, which the
base file already carries |
| `pnpm check:control-bytes` · `check:new-line-citations` ·
`check:changeset-claims` · `check:pending-changeset-literals` ·
`check:i18n-designer-parity` | 0 each | OK; 0 new citations; every en
row has a zh row with the same placeholders |
| `pnpm check:vi-mock-specifiers` · `check:vi-mock-inherit` ·
`check:vi-mock-override-shape` · `check:test-path-roots` | 0 each | OK |
| `pnpm check:i18n-keys` · `check:i18n-drift` · `check:esm-specifiers` ·
`check:self-import` · `check:phantom-deps` ·
`check:unreferenced-sources` | 0 each | OK |
| `node scripts/check-changeset-presence.mjs` · `pnpm changeset:check` ·
`node scripts/check-changeset-overwrite.mjs` | 0 each | 1 changeset
declared; no major; none overwritten |
| `pnpm check:handler-key-reads` · `check:metadata-write-doors` ·
`scripts/check-type-check-coverage.mjs` ·
`scripts/check-lint-coverage.mjs` | 0 each | OK |
| `node scripts/check-governed-queue-guard.mjs --test` (5 paths) | 0 |
NOT GOVERNED |
Test files: `ObjectFormDesigner.test.tsx`,
`ObjectFormDesigner.dndAnnouncements-11802.test.tsx`,
`DataPillar.hiddenSystemFields-11780.test.tsx`,
`StudioDesignSurface.formFields.test.tsx`,
`select-placeholder-literal-11252.test.ts`, `block-config-i18n.test.ts`,
`scripts/__tests__/check-i18n-en-drift.test.ts`,
`scripts/__tests__/check-i18n-dead-keys.test.ts`. `pnpm
check:i18n-dead-keys` (report-only) lists no `engine.studio.formDnd.*`
key.
The lint run is narrowed, not repo-wide: the population is app-shell's
own `eslint .` under the root `eslint.config.js`; `--format json`
counted 4 files; the config sets no `parserOptions` / `projectService`
(type-aware linting off), so this diff cannot move a verdict on an
untouched file. Repo-wide `pnpm lint` and the full test farm are CI's.
Changeset: `.changeset/11802-form-dnd-announcements.md`,
`@object-ui/app-shell` patch.
## Acceptance notes
Found while measuring, outside this card's fence, not fixed here:
1. **A keyboard drag never reaches a drop target.**
`collisionDetection={pointerWithin}` returns nothing without pointer
coordinates, and a keyboard drag has none (its activator is a keyboard
event), so `over` stays null. In the browser, *Industry*: Space,
ArrowDown three times, Space left the draft unchanged; before this
change the region said "Picked up draggable item f:industry." then
"Draggable item f:industry was dropped.". After it, the region says the
field is back in its place, which is true, but the instructions
(dnd-kit's default, now localized) still describe moving with the arrow
keys. The fix is a collision detection that also answers for the
keyboard. That changes drag behaviour, which this card rules out.
2. **Every field card announces `aria-roledescription="sortable"` in
English,** in zh too: the `useSortable` default in `SortableField`,
outside the `DndContext` props this card may touch.
The PM's report carries both, with their reach.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent bee5d6f commit 33e3583
5 files changed
Lines changed: 449 additions & 0 deletions
File tree
- .changeset
- packages/app-shell/src/views
- metadata-admin
- studio-design
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2940 | 2940 | | |
2941 | 2941 | | |
2942 | 2942 | | |
| 2943 | + | |
| 2944 | + | |
| 2945 | + | |
| 2946 | + | |
| 2947 | + | |
| 2948 | + | |
| 2949 | + | |
| 2950 | + | |
| 2951 | + | |
| 2952 | + | |
2943 | 2953 | | |
2944 | 2954 | | |
2945 | 2955 | | |
| |||
5962 | 5972 | | |
5963 | 5973 | | |
5964 | 5974 | | |
| 5975 | + | |
| 5976 | + | |
| 5977 | + | |
| 5978 | + | |
| 5979 | + | |
| 5980 | + | |
| 5981 | + | |
| 5982 | + | |
5965 | 5983 | | |
5966 | 5984 | | |
5967 | 5985 | | |
| |||
Lines changed: 245 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
Lines changed: 69 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| 59 | + | |
59 | 60 | | |
60 | 61 | | |
61 | 62 | | |
| |||
75 | 76 | | |
76 | 77 | | |
77 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
78 | 119 | | |
79 | 120 | | |
80 | 121 | | |
| |||
453 | 494 | | |
454 | 495 | | |
455 | 496 | | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
456 | 524 | | |
457 | 525 | | |
458 | 526 | | |
| |||
590 | 658 | | |
591 | 659 | | |
592 | 660 | | |
| 661 | + | |
593 | 662 | | |
594 | 663 | | |
595 | 664 | | |
| |||
0 commit comments