Repository navigation
Commit 60d11cd
fix(app-shell): form designer arrow keys reach every place on a multi-column canvas, and a keyboard drop commits the place it announced (objectui#11898) (#11940)
Fixes #11898
Clause-②: no. Keyboard geometry and the keyboard drag's drop target
inside the Studio form designer: no export, prop, `@object-ui/types`
member or accepted input changes.
## What changes
All in
`packages/app-shell/src/views/studio-design/ObjectFormDesigner.tsx`,
inside the claim's fence (the keyboard sensor's coordinate getter, and
`formCollision`'s keyboard branch):
- **The keyboard sensor's coordinate getter** is no longer dnd-kit's
`sortableKeyboardCoordinates`. A module-private `keyboardStep` walks the
designer's own layout (the container map `onDragOver` / `onDragEnd`
read) in reading order: ArrowDown and ArrowRight one place later,
ArrowUp and ArrowLeft one place earlier, whatever the column count. Past
either end of a group it enters the next group shown, first place going
down and last place going up, so an empty group is one step like any
other. At either end of the canvas it goes nowhere. It moves the chip
onto the droppable the drag is now over, so the canvas still scrolls to
follow it.
- **`formCollision`'s keyboard branch** answers the droppable that place
reads off the layout (`keyboardOverAt`): the card at that index in the
field's own group, the card it goes before in another group, the group's
section when it goes last, and the dragged card itself once `onDragOver`
has carried it there. This replaces `closestCorners` there. The pointer
branch is still `pointerWithin`.
- The place is held as state for the collision and as a ref for the
getter (the sensor keeps the getter from the pick-up on). It is reset on
every keyboard pick-up through the sensor's own `onActivation` option,
so nothing outside the sensor's configuration changes.
- **Unchanged:** `onDragOver`, `onDragEnd`, `dropPlaceIn`,
`formDndAnnouncements.ts`, `SortableField`, and the whole pointer path.
No handler change was needed: once the keyboard drag is over the drop
target its place names, the existing arithmetic commits the place the
live region announced.
New pin file `ObjectFormDesigner.keyboardGrid-11898.test.tsx`, and
`.changeset/11898-form-keyboard-grid.md` (`@object-ui/app-shell` patch).
## Measured first (real Chromium, before the fix, on `d52d908`)
The dispatch's three mechanism hypotheses, measured on a dev-only
harness: a Vite page mounting this `ObjectFormDesigner` (readOnly off,
sensors on) with objectui#11871's four-field fixture and an eleven-field
one (Contact holds Phone, Email, Mobile, Fax, the full-row Notes
textarea and Website). Playwright pressed the keys, a MutationObserver
recorded every live-region sentence, and a probe wrapping the stock
getter recorded its direction-filtered candidates with their
`closestCorners` values. The harness was never committed and is deleted.
- **H1, holds in part.** At 1280px, Email ArrowDown ranked `f:industry`
262, `g:new_group` 371, `g:__ungrouped__` 451, `f:name` 630, so the card
won and Email landed in Ungrouped at 2 of 3, skipping the empty group.
Name ArrowUp ranked `f:phone` 230 first (Contact 1 of 3). At 480px, Name
ArrowUp ranked its own section `g:__ungrouped__` 69 first; the getter
moved the chip to that section's corner, the drag stayed over its own
card, and nothing moved (case 3). **Correction for case 2:** the
full-row Notes' chip is as wide as its section, so Notes' OWN section
won ArrowUp (`g:contact` 163, then `f:email` 410). The drag was then
over the section, and the same-group arithmetic put Notes at the group's
END: "Notes moved to Contact, position 6 of 6", from 5 of 6. A Notes
that is already last reads as "does nothing".
- **H2, holds, with one amendment.** The reading-order getter reaches
every case at both widths (readings below). The amendment: the getter
alone is not enough. The keyboard collision has to answer from the same
layout, because a rect test misses the empty group even with the chip
placed on it (reverse-check leg 2 below).
- **H3, the mechanism holds, the place to fix it does not.** At 480px,
Phone ArrowDown ×3: the third step was over `f:name`, and the live
region said "Phone is over Ungrouped, position 1 of 3". `onDragOver`
carried Phone before Name. A probe at a no-op key right before the drop
read the drag still over `f:name`, and `onDragEnd`'s same-group branch
(old index 0, new index 1) committed "position 2 of 3". The announcement
is right, and the commit arithmetic is right for the `over` it is given.
The `over` was wrong: after the carry, the rect test under the chip
landed on the card the field had been carried before. With the keyboard
`over` following the layout (the dragged card itself after a carry, the
pattern of dnd-kit's own multi-container example), the existing
arithmetic commits the announced place. `onDragEnd` and the pointer
two-step objectui#11802 pinned are untouched.
## Real-browser reading after the fix (Chromium, HEAD blob
`c947f010ae64` of `ObjectFormDesigner.tsx`, both widths)
Identical sentences and commits at 1280px and at 480px:
| case | keys | live region after the keys, then the drop | committed |
|---|---|---|---|
| 1 | Email, ArrowDown | over New group 1 of 1 → moved to New group 1 of
1 | `new_group: [email]` |
| 1 | Name, ArrowUp | over New group 1 of 1 → moved to New group 1 of 1
| `new_group: [name]` |
| 2 | Notes (full-row), ArrowUp | over Contact 4 of 6 → moved to Contact
4 of 6 | `contact: [phone, email, mobile, notes, fax, website]` |
| 2 | Notes, ArrowDown | over Contact 6 of 6 → moved to Contact 6 of 6 |
`contact: [… website, notes]` |
| 3 | Name, ArrowUp ×2 | over New group 1 of 1, over Contact 3 of 3 →
moved to Contact 3 of 3 | `contact: [phone, email, name]` |
| 4 | Phone, ArrowDown ×3 | over Contact 2 of 2, over New group 1 of 1,
over Ungrouped 1 of 3 → moved to Ungrouped 1 of 3 | `'': [phone, name,
industry]` |
Also read at both widths: eleven fields, Phone ArrowDown ×8 and Owner
ArrowUp ×8 on a 420px-tall viewport. Every place is reached in order,
including Contact's end slot, the page scrolls with the chip, and the
drop commits the last place announced. A keyboard drag at either end of
the canvas commits the unchanged layout.
**Pointer control:** four pointer drags (Name onto the empty group,
Phone onto Email, Name onto Email, Name released outside every grid)
gave identical live-region sequences and commits before and after the
fix, at both widths.
## Pins
`ObjectFormDesigner.keyboardGrid-11898.test.tsx` runs the real
`DndContext`, `KeyboardSensor` and `PointerSensor`, the designer's
collision and handlers, with nothing in `@dnd-kit` mocked. Its layout
stub copies Chromium's geometry at both widths: the container-query
column count, a full-row card starting its own row, and the section
headers between grids. For each width it pins the four cases plus a
boundary case, each with the committed layout and the live region's
sentences. Case 4 also compares the last announced place with the
committed one. Controls: the one-column keyboard move objectui#11871
already reached (Email ArrowDown at 480px), and two pointer drags on the
two-column canvas.
## Reverse checks (WRAP mode of objectstack's
`scripts/ablation-replace.mjs`, inside the verify lock, committed head
`8c1b262`)
The tests import `./ObjectFormDesigner` from source, so there is no dist
leg. Every leg's restore was proven by `blob c947f01 == HEAD` and
an empty `git diff HEAD`.
- **Leg 1, the base mechanism back** (`sortableKeyboardCoordinates` as
the getter and `closestCorners` as the keyboard branch; 4 anchors, each
1 → 0): **9 failed / 13 passed**. Red: cases 1, 2, 3 and 4 at 1280px,
and Name ArrowUp, case 3 and case 4 at 480px, each with Chromium's own
pre-fix sentence. For example, 1280px Email read "over Ungrouped,
position 2 of 3", and 480px Phone committed "position 2 of 3" after
"over … 1 of 3". objectui#11871's three-step pin also went red. Green:
the 480px Email move and the 480px Notes moves (Chromium reached those
before the fix too), the boundary cases, and the pointer controls. The
stub reproduces the pre-fix Chromium readings.
- **Leg 2, the keyboard collision only back to `closestCorners`** (new
getter kept): **8 failed / 14 passed**, case 4 red at both widths
("position 2 of 3" committed at 480px), and cases 1 to 3 red at 1280px
(Email ArrowDown over "Contact, position 1 of 2": the empty section
loses on corner distance even under the chip).
- **Leg 3, the stock getter only** (new collision kept): **15 failed / 7
passed**. Every keyboard move stays put, the one-column control
included, because nothing records a place. The pointer controls and the
boundary cases stay green.
## Gates (on the merge head `a06a2a4`, origin/main `f1781be` merged in,
no rebase)
- `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...'
build` → 0 (28 packages)
- `pnpm exec vitest run` over the six `ObjectFormDesigner*` /
`StudioDesignSurface.formFields` files plus
`viewCacheInvalidation.guard.test.tsx` → 0, 7 files / 60 tests passed
- `pnpm exec vitest run packages/app-shell/src/views/studio-design/` →
0, 136 files / 854 tests passed
- `pnpm --filter @object-ui/app-shell type-check` → 0 (`tsc -p
tsconfig.test.json --listFilesOnly` lists both pin files)
- `pnpm check:control-bytes`, `check:new-line-citations`,
`check:changeset-claims`, `check:pending-changeset-literals` → 0 each;
derived from this diff: `check:test-path-roots`,
`check:vi-mock-specifiers`, `check:vi-mock-inherit`,
`check:vi-mock-override-shape`, `check:phantom-deps`,
`check:unused-deps`, `check:unreferenced-sources`, `check:i18n-keys`,
`check:shell-escape-residue`, and `scripts/check-changeset-presence.mjs`
/ `-no-major` / `-fixed` / `-overwrite` → 0 each
- ESLint, narrowed: `eslint --no-inline-config --format json` on the 3
changed source and test files → 3 files, 0 errors, 1 warning
(`react-hooks/set-state-in-effect` on the untouched draft re-sync
effect, present on `main`). Population: the root `eslint.config.js` as
`--print-config` resolves it. Invariance: no `parserOptions.project` /
`projectService`, so no type-aware rule can move an untouched file's
verdict. The repo-wide `pnpm lint` and the full test farm are CI's.
## A conflict in the dispatch, said here rather than chosen silently
The dispatch rules that objectui#11871's pins "stay green as they are".
One of them, "carries a field up through the empty group into a group
that has fields", pinned this card's case 3: its first ArrowUp from
Name, the first field of Ungrouped, moved nothing. Fixing case 3, which
the same dispatch rules in, changes that pin's intermediate sentences.
They now read one place per step: over New group 1 of 1, over Contact 3
of 3, over Contact 2 of 3. Chromium reads the same after the fix. Its
final sentence and committed layout are unchanged, and every other
objectui#11871 pin (pointer controls included) and every objectui#11802
pin stays as it was. The pin's header line that named
`sortableKeyboardCoordinates` as its instrument now says "the designer's
own coordinate getter".
## Acceptance notes
- ArrowDown is "next place in reading order", not "next row", on a
multi-column canvas: one step is one position, which is what the live
region speaks. ArrowRight and ArrowLeft step the same way.
- If a field is the only one in Ungrouped and a keyboard drag carries it
into a group, the designer hides the now-empty Ungrouped bucket
mid-drag, so ArrowDown cannot bring the field back there. Escape cancels
the drag. A pointer drag behaves the same, because the section is gone.
- The pending objectui#11871 changeset says the designer uses
`closestCorners` for a keyboard drag. This PR replaces that branch, and
its changeset says so, so the two entries read in order in the next
CHANGELOG. That changeset file is outside this claim and was not
touched.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 2fec2e0 commit 60d11cd
4 files changed
Lines changed: 485 additions & 23 deletions
File tree
- .changeset
- packages/app-shell/src/views/studio-design
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 5 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
207 | | - | |
208 | | - | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
209 | 210 | | |
210 | | - | |
211 | 211 | | |
| 212 | + | |
212 | 213 | | |
213 | 214 | | |
214 | 215 | | |
| |||
Lines changed: 351 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 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
0 commit comments