Repository navigation
EPBDS-13509: a highlight-N-last table highlights its own last rows, not an expanded row's content or a nested table - #238
Merged
Conversation
… not an expanded row's content or a nested table The rule in `table.less` counted the last rows of every `tbody` inside the table, and counted the row an expanded row shows its content in: - expanding the last row of a `highlight-1-last` table moved the highlight from it to that content row, so all the content, nested tables included, was drawn bold; - the last row of every table nested in a highlighted one was highlighted too. The selectors now start at the table (`> tbody > tr`) and skip the content row: `:nth-last-child(-n+N of :not(.app__table__expansion))`. `TableView` gives the content row that class; it joins `class-inventory.txt`, and the DOM snapshots of `all` and `nestedDataKind` gain it on their five content rows. The highlight's background is left as it is: cells have had a white background of their own since January 2023, which covers it, and the owner chose to keep only the bold weight.
…nested-highlight-abc974 # Conflicts: # src/demo/markdowns/changelog.md
…nested-highlight-abc974 # Conflicts: # e2e/interactive-views.pw.js # e2e/view-coverage.js # src/demo/markdowns/changelog.md
This was referenced Oct 23, 2026
AlexSamBY
added a commit
that referenced
this pull request
Oct 23, 2026
## What A table row with a form of its own registers its row's place as its meta's `relativePath` and `relativeIndex`. That is the add row a `renderExtraItem` declaration renders, or a row a `useForm` declaration gives a form. Its fields are named by that place (`dataKind.items[1].kind`), so its values hold the row there already. `getFormsData` and `getRawFormsData` placed them there a second time, at `dataKind.items[1].dataKind.items[1]`. Measured on the code before this change: - a field the add row gated with a `showIf` on another of its fields never showed; - an edit in a `useForm` row put a copy of the table inside that row in `getFormData`, and a `showIf` in the row read the row as it was; - while a table in another table's rows had an add row with a form, in a document with more than one top-level key, what a `showIf` reads held none of the outer rows. Both readers now go through `mergeForms` (`formData.ts`), which places such a form where its fields put it: - **Only the row.** The rest of its values is what it started from, the row's data or the declared `initialValues`, at the root of its values, where none of its fields reads. - **Into the document's own containers.** In containers of its own, the place of a table nested in a row (`dataKind.phases.0.dataKind.lineItems[1]`) made `phases` an object, which the merge put in place of the document's array. - **Last.** Forms register children first, and `mergeData` finds no master in a document with one top-level key, so the document's copy of an edited row came last and won. Every other reader already took a row form's values as placed: `addData` (`rowObjectForDataKindAppend`), the live merge (`getLiveMergedDataKindArray`) and the error report. The add row is still left out of `getFormData`, Apply, upload and submit (`dataFormsOf`). ## A `useForm` row in a table It had the same doubled path, which this fixes. Two defects of its own are left as they were, measured: - Its fields start empty. Its form starts from the row's values at the root of its own, and its fields read the row's place. - A submit after an edit in it sends the document's copy of the rows, up to the edited one. `getLiveMergedDataKindArray` lets the later form win, which is the document's, and caps the rows at the shortest array, which a row's form holds up to its own index. ## Tests - `rules.row-form-values.test.js`, through `UIRender`: - the add row's `showIf`, before and after an Add; - a `useForm` row's edit, in `getFormData` and in its `showIf`; - a nested table's add row, and the outer rows' `showIf`. All six fail on the code before this change. - `formData.test.js`: the placement, for both readers. Merging the rows before the document fails the precedence tests, here and in the `useForm` row's. - Two unit tests pinned the old placement with values no row form holds, the row's fields at the root of its values. They now hold them at the row. - `formData.test.js` also covers the live merge merging again when only a later form holds a new array. The placement code that went took covered branches with it, and `formData.ts` keeps its 98% branch threshold with this one. ## Gates On Node 24.14.0, on the branch merged with `origin/master` (#237, #238), these exit 0: - `npm test` (240 suites, 3073 tests) and `test:coverage`, with every threshold held; - `test:react16`, `test:react17` and `test:react19`; - `lint:js`, `typecheck`, `docs:views:check`, `docs:props:check` and `docs:phrases:check`. The jest legs ran with `<rootDir>/\.claude/` ignored, for a stray, gitignored probe in this checkout. The render budgets pass unchanged, and no snapshot changed. The merge conflicted only in the changelog, where both sides' entries are kept. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
AlexSamBY
added a commit
that referenced
this pull request
Oct 23, 2026
…id date` (#239) ## What EPBDS-12884: a date typed as `11-31-2021` or `02-29-2021` was lost without a word. Measured in Chromium on the demo's `tableForm` (Table with Form Inputs), dates as `MM-DD-YYYY`: - in an empty required field, the text was wiped once the field was left, and the field said `Required`; - typed over a date, the text was replaced by that date again, and nothing was said. rc-picker reads what a user types strictly, in the field's formats, and hands on only a date it reads. When the field was left, rc-picker showed the date it had again, or nothing. Its `preserveInvalidOnBlur` keeps the text, and does nothing else: no error, no change of value. The owners' calls, made for this ticket: 1. **While the text is no date, the value is the date the field had, or none.** That is what `getFormData` returns. A submit, an `onApplyPeriods` and a table's Add send nothing while the error shows. A typo does not reach `onChange` or `autoSubmit`. 2. **The text stays in the field after it is left.** 3. **It is an error in an optional field too.** How: - **`InputDate`** keeps the text (`preserveInvalidOnBlur`). It reports, through a new `onInvalidText`, the text a field is left with when that is no date in its formats, and null once the field shows a date or nothing again. It learns what the input shows from the `<input>` it hands rc-picker as `components.input`: the same element, which reports each text it shows. rc-picker keeps that text in its own state and changes it as the user types, as the value changes, and as a day is hovered. - **`asInputDateField`** gives every date field a validator around its own, which says `Invalid date` while the field shows such text. The value does not change, so final-form would not validate again: the field asks the form to (`revalidate`, a new export of `batchedForm.ts`, which does what `setConfig('validate', …)` does there already). The validator keeps the arity of the field's own, because final-form computes the field's state only for a validator with three parameters or none. - **The `reset` action** shows the value again in place of the text. `countReset` in `state/formRegistry.ts` counts it, and a field drops text it was left with before the last reset and renders a new input. Only the action counts. The form layer also resets a form to new initial values that a host or a parent document hands over, and there a field whose value they left unchanged keeps its text. - **A `localDraft` row** holds Add back with `Invalid date` the same way. - **`Invalid date` is a built-in phrase**, `INVALID_DATE`, which a host translates as `ui-render:INVALID_DATE`, as #240 made of every validation message. The field and the `localDraft` row return `phraseMessage('INVALID_DATE')`, made once, and it is in `BUILT_IN_PHRASES`, the published `UIRenderPhraseId` and `docs/BUILT-IN-PHRASES.md`. Two more ways the field lost what was typed, found while measuring. Keeping the text after the field is left would have made both worse, so both are fixed here: - **A valid date typed and left by a click was dropped.** Only Tab and Enter kept it. A click on the header, on another field or anywhere else put the old date back. `InputDate` handed rc-picker a new moment of the same date on every render. The field renders again when it is left, because it is touched. rc-picker then reset the date it had read and not confirmed yet, a frame before it would have confirmed it. Now there is one moment per value (`useMemo`). Measured on a scratch build before the rest of the change: every way of leaving keeps the date. - **Emptying a date's text did not empty the date.** Leaving the field showed the date again. Now the date goes with the text, and a required field says `Required`. rc-picker still confirms a date it read before the text was emptied, once the field is left. Until the next focus, the field takes none. Measured in Chromium on a production build with this change, both ways of leaving (a click and Tab): `11-31-2021`, `02-29-2021` and `11/31/2021` stay in an empty required field with `Invalid date`. Over a date, `02-29-2021`, `11-31-2021` and `11-3` stay with `Invalid date`, and Submit sends nothing. `11-30-2021` is taken and submitted. An emptied field says `Required`. Correcting the text, or picking a day in the calendar, takes the error away. ## Not changed - Text in another shape than the display format, such as `11/30/2021` for `MM-DD-YYYY`, is not read as a date: it says `Invalid date` now, where it was dropped before. Reading other separators would be a feature. - rc-picker's own `aria-invalid` and red outline (`.ui-render-picker-invalid`) still follow what was last typed. After a day is picked in the calendar in place of the text, they stay until the next keystroke, as they did on master after the text was put back. - If the text is a date for a moment while it is typed and then no date, as `11-30-2021` and then `11-30-202`, rc-picker confirms the date it read once the field is left. The field shows the text and `Invalid date`, and its value is that date. On master, Tab kept that date and showed it, and a click put the old date back. - A date field that unmounts (a tab, a `showIf`, a collapsed row) forgets the text, and shows its value when it comes back. ## Tests - `InputDate.typed-text.test.js`: typing through rc-picker itself in jsdom. - Text that is no date stays and is reported, and no date changes: in an empty field and over a date. - A date left without Tab or Enter is taken, from a field that renders again when it is left. - Corrected text is reported as no longer invalid, and its date is taken. - A new value shown in place of the text is reported. - An emptied field: its date goes, and a date read earlier is not taken, whether the field had a date, had none, or was left by Tab. - A date read while the text was typed, then text that is no date: the date is taken, the text stays. - The text stays where nothing listens for it. - `rules.invalid-date.test.js`: through a document with a period table. - `Invalid date` in an empty required field, for each of the three texts. - Over a date, `getFormData` still returns the date. - `Invalid date` in an optional field too. - Submit and Apply send nothing, and send the corrected date. - Add is held back. - The `reset` action shows the value again. - The text survives an edit beside it. - A date left by a click is submitted. - An emptied field says `Required`. - `LocalDraftTableRow.test.js`: Add is held back with `Invalid date`, in an optional cell too, and adds once the text is corrected. - `built-in-phrases.test.js` (from #240): a date field left with text that is no date shows the host's German for `ui-render:INVALID_DATE`, and `Invalid date` for a host with no translation or no `translate`. - `e2e/interactive-views.pw.js`, "date input: what a user types": the two reported cases with both texts, a date left by a click, a corrected text, an emptied field. Each leaves by a click. All 5 fail on a master build and pass on this one. - Each part was taken out once, and a test failed each time: the memo, `preserveInvalidOnBlur`, the emptied flag, `revalidate`, `countReset`, the new input after a reset, the validator of a field without its own, and the draft row's check. ## Checks On this branch merged with master `001912cd` (#242), each judged by its exit code, all 0: - `npm run typecheck`, `npm run lint:js`, `npm run docs:phrases:check`, `npm run docs:props:check`. - `npm run test:types`, which builds the library and compiles the consumer fixtures against `dist/*.d.ts` under `@types/react` 16 to 19. Its fixture registers a text for every published phrase id, `INVALID_DATE` included. - `npm test`: 3186 passed, 1 skipped. - `CI=1 npm run test:e2e`: 75 passed, Chromium desktop and touch, on a build it made itself. On the merge with `4bb35c7b` (#241), the step before: `npm run test:react16` and `test:react17` (3182 passed), `npm run test:react19` (3187 passed) and `npm run test:coverage`, with its global and per-file thresholds. The render budgets (`src/library/__tests__/*-budget.test.js`) assert exact counts, and they are unchanged. In the worktree, jest ran with the config's ignore patterns plus the checkout's own `.claude/` folder, which holds a local test file outside the repository. Earlier, on the merge with `fdfd09fe` (#238): `npm run test:pack` (budgets: 26 of 28 files, 3.45 of 4.00 MB unpacked, 0.90 of 1.00 MB packed) and `npm run test:pack:browser` (5 passed). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
AlexSamBY
added a commit
that referenced
this pull request
Oct 23, 2026
… highlight (#244) ## What A table with `highlight-1-last`, `highlight-2-last` or `highlight-3-last` and an add row (`renderExtraItem`) highlighted the add row. The add row comes after the table's rows, and #238's rule left out only the content row of an expanded row. Measured on master in Chromium, on the demo page, with a table of two rows and an add row, as the `tableForm` example has: | | first row | last row | add row | |---|---|---|---| | `highlight-1-last`, master | 400 | 400 | 500 | | `highlight-2-last`, master | 400 | 500 | 500 | | `highlight-1-last`, this branch | 400 | 500 | 400 | | `highlight-2-last`, this branch | 500 | 500 | 400 | A `localDraft` add row behaved the same, and is fixed the same way: `TableView` renders both in one row. - **`TableView.tsx`:** the add row gets the class `app__table__add-row`, which joins `src/style/class-inventory.txt`. - **`table.less`:** the selectors leave it out as they leave out `app__table__expansion`: `> tbody > tr:nth-last-child(-n+N of :not(.app__table__expansion, .app__table__add-row))`. `public/static/ui-render.built.css` is rebuilt. - **Docs:** a Fixes entry in `src/demo/markdowns/changelog.md`, and the note in `styles.md` names the add row. `striped` selects its rows the same way (`tr:nth-child(even)`, descendant), so its stripes reach nested tables and count the expansion and add rows. Nothing of it shows: the cells' white background covers the rows' (as #238 found for the highlight's background), so this PR leaves it alone. ## Tests - **Browser (`e2e/interactive-views.pw.js`):** `[I] a table highlights its own last rows, and not the add row after them`, with `highlight-1-last` and `highlight-2-last` (`HIGHLIGHTED_ROWS.WITH_ADD_ROW` in `e2e/reference.js`), listed in `e2e/view-coverage.js`. - **jsdom (`TableView.test.js`):** the add row, and no other, carries the class. - **DOM snapshots:** `examples.dom-contract` updated on purpose: 5 lines, the class on the add rows of two examples. ## Checks On `f31285ea`, master `53e66a2f`, all exit 0: - `npm test`: 243 suites, 3183 tests, 5 skipped. Run without the worktree's gitignored `.claude/` probe, and with `/usr/bin` first in `PATH`: this Mac's `/usr/local/bin/git` no longer starts, and the manifest contract spawns git. - `lint:js`, `lint:css`, `typecheck`, `build-css:check`, `docs:props:check`, `docs:views:check`. - `CI=1 npm run test:e2e`: 76 passed, Chromium desktop and touch. - The new browser test and #238's pass in WebKit. Firefox did not start on this Mac ("Could not find profile folder"), so the `browser-engines` job is the one that runs them there. Not run here, left to CI: the React 16, 17 and 19 legs, `test:types` and the pack legs. The change is one class name and three selectors. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
EPBDS-13509. A table declared with
styles: 'highlight-1-last'(orhighlight-2-last,highlight-3-last)highlighted the wrong rows once it had a row expanded or a table nested in it. Measured on master in Chromium, on the
demo page, with the ticket's meta (a
categoriestable whose rows expand into a title and aclassestable) andarray-nested_data.json:own below it (
renderIteminTableView.tsx), and the rule'str:last-childwas then that row. Silver, the lastrow, went from weight 500 to 400, and the content row, with everything in it, from 400 to 500.
tbodywas a descendant in the selector, so the last row of every tableinside a highlighted one matched as well.
The selectors now start at the table and leave the content row out:
TableViewgives the content row the classapp__table__expansion, and the class joinssrc/style/class-inventory.txt.Browser support. All 22
browserslisttargets support:nth-last-child(An+B of S)(caniuse-lite'scss-nth-child-of, checked per target). A browser without it, such as Chrome before 111 or Firefox before 113, dropsthe whole rule and shows no highlight; none of them is a target. The minified
static/all.cssthatbuild-libpublishes keeps all three selectors, and Chromium, Firefox and WebKit parse them and compute the expected weights.
The background stays as it is. The rule also gives the row a 2.5% grey background, which has not shown since
January 2023: the change that let columns be pinned gave cells a white background of their own, and it covers the
row's. Asked, the owners chose to keep only the bold weight, so this PR leaves the background alone.
Measured
Chromium, the demo page, the ticket's meta and data. The font weight of each row:
Firefox and WebKit measure the same as Chromium on this branch.
Tests
e2e/interactive-views.pw.js, one new case. It is listed underTableine2e/view-coverage.js, and itsvalues are
HIGHLIGHTED_ROWSine2e/reference.js.upload zones.
highlight-1-lastwith the last row expanded, andhighlight-2-lastwith thefirst row expanded, where the content row sits between the two rows the table counts. Before and after the row
expands, it reads the weight of every row of the table and of the nested table.
table's row 500.
TableView.test.js, one new case. The content rows carry the class, and no other row does. It fails withoutthe class.
allandnestedDataKindrender expanded rows. Theirfive content rows gain
class="app__table__expansion", and nothing else moved. The draft row ofnestedDataKind(
renderExtraItem) does not get the class.npm run build-cssregeneratedpublic/static/ui-render.built.css: the one rule.Not changed
highlight-1-lasttable with arenderExtraItemrow, that row takes the highlight, and the last row of data does not. Measured on this branch; the ticket does not
cover it.
stripedhas the same selector shape. It stripes every even row inside the table, the nested tables' and thecontent rows included. The cells' white background covers its stripes as well, so nothing shows either way.
Gates
On HEAD, with master merged in, on Node 24.14.0, all exit 0:
npm test: 239 suites, 3055 passed, 1 skipped.test:coverage;test:react16andtest:react17(3051 passed,5 skipped);
test:react19(3056 passed).lint:js,lint:css,typecheck,build-css:check,docs:views:check,docs:props:check,docs:phrases:check.build-lib, thentest:css:built.test:e2e: 67 passed, Chromium desktop and touch.Two notes on how they ran.
.claude/. This worktree holds a gitignored probe,.claude/probe-dim5/nodes.test.js,which fails on its own; CI's checkout does not have it. The runs passed
<rootDir>/\.claude/totestPathIgnorePatterns, with the config's own patterns.test:e2eran withCI=1. Another worktree's server was on port 3199 earlier, and withoutCIPlaywrightreuses whatever answers there. With it, the run built and served this branch.
Not run:
test:types,test:packand the pack, env-flag and perf legs. This change touches no types, packaging orenv flags.
🤖 Generated with Claude Code