feat(grid): expand every nested object, per document and page-wide - #166
Merged
Merged
Conversation
The list view's per-line chevrons only ever moved one level, which is the wrong granularity for the documents people actually read. A `processInfo` keyed by device id, each entry an object of its own, took one click per level to reach — and then the same clicks again in the next document, and the one after that. Reading a page of them was not a practical thing to do. Two controls, because the right scope genuinely differs by case: - **Per document**: a chevron in each card's header, beside the field count, opening or folding every nested object in that document at any depth. Always visible rather than revealed on hover — it answers "what is actually in here", which is the question you have *before* you know whether the card is worth pointing at — and hidden entirely on a document with nothing to unfold. - **Page-wide**: the same pair in the grid footer, next to the column-fit and row-zoom controls. That row is already "how am I looking at this", and the fit controls it sits beside are its exact table-view counterpart: both are gated to the view where they mean something. Three implementation decisions are worth recording, each having had a plausible-looking alternative: **It flips the base the folds are a diff from, not a set of paths.** A container hidden inside a folded ancestor contributes no line, so its path is absent from the flattened field list and there is nothing to toggle — a set-based "expand all" would have unfolded exactly one level and stopped. `DocumentCard` already stored folds as a diff from `listExpandNested`; moving the base that diff is measured against opens the whole tree in one move, at any depth, at no cost per level. **The page-wide press is an epoch, not a boolean.** It is an action, not a state: after pressing it the user may fold one object by hand, and pressing it again has to expand that object back. A boolean prop would already be `true` on the second press and nothing would happen. The epoch is also what lets a card that scrolls into the virtualizer's window *after* the press mount already expanded — off-screen cards are unmounted, so a fresh one has no history of its own to go on and reads the signal at mount instead. **Neither control writes the `listExpandNested` preference.** That one answers "how should a document open"; these answer "show me everything in what I am looking at right now". Conflating them would make a one-off gesture rewrite a persisted setting, and would also be a lie: the preference is a *default*, and a card that has been expanded wholesale still yields to it the moment it changes at runtime (the override resets, which is what keeps the existing runtime-flippable semantics intact). The per-document control's direction follows what is on screen — it reads "collapse" only once every *visible* container is open — so it never disagrees with what the user is looking at. Also fixes a latent gap in `DocumentListView.test.tsx`: `globals` is off in `vitest.config.ts`, so testing-library never registers its automatic cleanup and the file's own `afterEach` was not calling it. No existing test noticed (none used `screen`), but every new query would have found two of everything. Authored by Alex López (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… StrictMode discards
The footer's "expand every nested object" reached every card and did nothing,
while the identical action on a card's own header worked. Instrumenting both
ends showed the state was flowing perfectly: the grid had counted 21 presses
and each card reported `e21T·seen21·baseF` — it had received the newest
gesture, marked it as seen, and never moved its base.
That fingerprint rules out every wiring explanation (a memo bailing out, a
prop not threaded through, a stale module, a remount) because all of them
predict a card that has *not* seen the signal. What produces it is the
marker itself:
const lastEpoch = useRef(expandAll?.epoch ?? 0);
if (expandAll && expandAll.epoch !== lastEpoch.current) {
lastEpoch.current = expandAll.epoch; // survives the discard
setBaseExpanded(expandAll.expanded); // discarded with the render
}
`main.tsx` wraps the app in `StrictMode`, which invokes a component body
twice and throws the first pass away — but a ref mutation is not part of
what gets thrown away. Pass 1 advanced the ref and queued the update; React
discarded that update and ran the body again; pass 2 compared against the
already-advanced ref, found nothing to do, and queued nothing. Hence seen
and not applied.
Setting state during render stays — it is the right tool, and an effect
would paint one frame with the stale folds before correcting itself. What
changes is where the comparison value lives: React's "adjust state when a
prop changes" keeps the previous prop in `useState` for precisely this
reason, because state is discarded alongside the render that set it, so the
second pass sees the same "before" value the first one did. The neighbouring
"let the preference win over a stale override" block had the same defect and
the same fix; it had simply never been pressed twice in one session.
Why the per-document button was fine all along: an `onClick` is not a
render, runs once, and has no marker to get ahead of itself. One line of
shared state produced a bug that looked like a fault in the page-wide path
and was not.
Tests: the four cases covering this gesture all passed against the broken
build, because outside `StrictMode` the body runs once and pass 1 *is* the
render — they were agreeing with the bug. `DocumentListView.test.tsx` now
mounts every one of them in `<StrictMode>`, exactly as `main.tsx` does;
reintroducing the ref fails "applies the grid-wide gesture, and re-applies
it on a second press". Recorded as ADR gotcha #85, with the rule that any
component adjusting state during render belongs under `StrictMode` in its
test.
Authored by Alex López (Alexfp28) <alexlopezdelafuente@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The aggregation preview is the one place the list view is mounted without a `DataGrid` around it — it is a stage card's right-hand pane and the text mode's output — so the grid footer's "expand every nested object" never reached it. Its documents are exactly the ones that need it: a `$lookup` or a `$group` with an accumulated array produces output that nests by construction, and reading it meant one chevron per level, per document. Three decisions, each with an alternative that looked reasonable: **The control floats over the documents rather than taking a bar.** A permanent strip would cost preview rows in a pane whose whole purpose is preview rows — the stage card splits its width between the editor and this, and vertical space there is already the scarce thing. It is revealed on hover for the same reason the cards' own row actions are, and anchored clear of the scrollbar. **It owns its own `ExpandAllSignal`.** The grid holds that state because the footer that drives it belongs to the grid; here there is no grid, and threading a signal down from `AggregationTab` would make two call sites (`StageCard` renders one preview per stage) share a gesture that is per-pane. Same epoch semantics as the grid's, for the same reason recorded in gotcha #85. **The control hides when nothing nests, and asks only the first 20 documents.** A `$group` projecting four scalars has nothing to unfold, and a button that cannot do anything is worse than no button. The check runs on every preview refresh — a debounced keystroke in the stage body — and the question is a yes/no about shape, which a sample settles as well as a scan; a pipeline whose 40th document is the first to carry a sub-document loses the button, which is the cheaper of the two mistakes available. `PipelineOutput.test.tsx` is new and covers the wiring this surface owns (the shared half — what a card does with an epoch — stays in `DocumentListView.test.tsx`). It mounts under `StrictMode`, as everything touching this gesture now does. Authored by Alex López (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Authored by Alex López (Alexfp28)
<alexlopezdelafuente@gmail.com>Qué
Desplegar todos los objetos anidados de la vista de lista, en tres alcances:
Los chevrons por línea solo movían un nivel, que es la granularidad equivocada para los documentos que se leen de verdad: un
infou otro subdocumento costaba un clic por nivel, y otra vez lo mismo en el documento siguiente.Cómo
listExpandNested. Esa decide cómo se abre un documento; estas responden a "enséñame todo lo que estoy mirando ahora".El bug que se comió el segundo commit
La versión inicial llegaba a las tarjetas y no hacía nada: el marcador del último epoch aplicado vivía en un
useRef, yStrictModeejecuta el cuerpo del componente dos veces descartando la primera pasada — pero una mutación de ref no se descarta con ella. La pasada 2 leía "ya aplicado" y el únicosetStateencolado pertenecía al render que React tiró. Instrumentado se veíacard:e21T·seen21·baseF: visto y sin aplicar, que es una huella que ninguna hipótesis de cableado predice.El arreglo es el patrón documentado de React: el valor anterior va en
useState, no en un ref. Los cuatro tests que cubrían el gesto pasaban contra el build roto porque no montaban enStrictMode; ahora lo hacen todos, y reintroducir el ref los rompe. Queda registrado como ADR gotcha #85, con la regla de que todo componente que ajuste estado durante el render se testee bajoStrictMode.Verificación
pnpm exec tsc --noEmitlimpio;pnpm exec vitest run— 102 ficheros, 1210 tests en verde.CHANGELOG.md+CHANGELOG.es.md) ydocs/MONGODB.md+docs/MONGODB.es.mdactualizados.