Add analysis-keyed mutations (edit, merge, delete) - #286
Draft
alex-rawlings-yyc wants to merge 13 commits into
Draft
Add analysis-keyed mutations (edit, merge, delete)#286alex-rawlings-yyc wants to merge 13 commits into
alex-rawlings-yyc wants to merge 13 commits into
Conversation
The catalog panel listed every analysis in one fixed order with no way to narrow it, so a draft of any size was only navigable by scrolling. The query core already supported all of this; only the UI that varies it was missing. Search, sort, and filter state is ephemeral useState inside the panel. The panel is mounted only while open, so closing it clears the query — a filter that survived a reload would leave rows missing with nothing on screen saying why. Filters sit behind one control that reports how many are active, so a panel narrow enough to need filtering is not itself filled with them. All four groups ship: the facet-derived ones (books, part of speech, confidence, and each named feature), missing gloss, breakdown, and unused-only. Against today's data only books raises a control, since no write path records the others yet — the facets are rightly absent rather than offering a lone choice. Facets are derived from every row rather than from the rows a filter left standing, so a selection cannot collapse the facet that would widen it back. The new useRowWindow mounts a growing leading slice of the listing, extending as the end comes into reach and starting over when the query changes. It is deliberately not useSegmentWindow: a row list has no counterpart to the scripture reference that hook holds still, so it needs none of that geometry bookkeeping. A listing narrowed to nothing now says so, rather than reusing "No analyses recorded yet" and telling readers their draft is empty when they have merely mistyped. That message and the panel's original one both go through the platform EmptyState. Stubs the platform SearchBar, Select, MultiSelectComboBox, and EmptyState, each documenting where it diverges from the component it stands in for.
The sort option substituted the raw book code into "Most used in
{book}",
so the dropdown read "Most used in GEN" while the row column beside it
resolved the same book through Canon.bookIdToEnglishName and read "Uses
in
Genesis" — one book named two ways in one open panel.
Resolve the name once in the panel and pass it to both views, so the two
labels cannot disagree. CatalogQueryControls takes the resolved name
rather
than the code, which keeps it presentational and leaves book-name
resolution
in the panel.
The test rerendered through a bare InterlinearNavProvider rather than the PanelProviders root it mounted with. React saw a different element type at that position and remounted the provider, reinitializing the ref that holds the pending request — so the closing assertion found no request because none had survived the remount, not because navigating past EXO had abandoned one. Deleting the abandonment effect entirely left the test green. Rerender through PanelProviders instead, keeping the provider that owns the request mounted across both navigation steps. The test now fails with "EXO 3:14:8" when the abandonment effect is removed. Also lift the two collators out of the query memo. They were rebuilt on every keystroke in the search box, which changes the query but neither language tag.
Two identical declarations shadowed each other, and neither tsc nor ESLint covers __mocks__, so nothing flagged it.
The stub group seeded its layout from `defaultLayout` unconditionally, where the real group takes that prop only when it names exactly the panels mounted and discards it otherwise. A layout naming a closed panel therefore came back by itself here on the next mount, which upstream would have thrown away. That divergence hid the loader's restoring effect: the width it exists to reapply was already in the group's state before it ran, so the effect could be made a no-op with every catalog test still passing. Seed after the panels have registered instead, matching on the count as the real group does. Cover the effect with a test that mounts the group closed, leaving it knowing only of the view so the stored layout reaches it only by being applied as the catalog's panel joins. Disabling the effect now fails five tests.
The window reset exists for a changed query: a reader who narrows a listing is looking at a new list, not further down the old one. It was keyed on the rows array's identity instead, which is only a proxy for that — and a proxy that also turns over on any edit to the underlying analysis. A gloss approved in the view beside an open catalog therefore collapsed a deeply scrolled list back to its first chunk, throwing the reader to the end of forty rows while the sentinel re-extended beneath them. useRowWindow now takes the query itself and compares that by reference, so the hook does what its own doc comment already claimed. Also withhold the query controls from a draft that has recorded nothing, where a search box, sort and filter popover narrow an empty listing and the popover in particular is an invitation to a dead end. The gate reads the draft rather than the queried rows, so a query that matched nothing keeps the controls that are the only way to widen it back.
A feature value is free text, so it may be the empty string. The filter offered such a choice under its own spelling, which the platform control can neither carry as a value nor show as a label: the choice appeared as a blank, unclickable row. Give it a sentinel of its own, alongside the one the absent choice already uses, and a localized label to be read under. Route the three places that spelled a choice for the control through one function, so they cannot disagree about the two choices that need spelling. Latent for now, no write path recording feature values yet.
The catalog's rows are shared TokenAnalysis payloads with no token in context, so the existing tokenRef-keyed reducers cannot serve them: those fork a shared payload to keep an edit local to one token, which is the opposite of what editing a catalog row means. Add a second reducer family keyed by analysisId — writeAnalysisGloss, writeAnalysisMorphemes, writeAnalysisMorphemeGloss, deleteAnalysis, and mergeAnalysisInto — that never forks. The key alone determines the blast radius, so neither family needs a scope flag. Each write re-converges onto a content-identical sibling as the per-token path does. Add selectAnalysisDeletionOutcome, which reports whether deleting a row leaves its tokens blank or falling back to a surviving homograph, so the confirmation can name the concrete consequence. It rebuilds the pool with the payload dropped outright rather than discounting a single approval, which would leave a multi-token payload competing to replace itself. Add selectAnalysisMergePeers to offer merge targets from the row's own pool bucket, so merging is confined to genuine homographs.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reconcile the filter selections against the facets each render, so a selection cannot outlive the choice it names: an edit beside the panel that empties a facet no longer strands the listing behind a control that is off screen. Distinguish a placeholder label from a real value reading the same, the platform combo box resolving a selection by its label alone. Carry an unchanged morpheme across a re-split whole, keeping the id its references depend on along with its gloss and lexicon refs. Count the deletion preview's usages off the approved-token index the catalog row counts by, as its own doc already promised.
Merge peers came off the suggestion pool, which admits approved analyses alone, so an imported homograph was listed as a catalog row that could neither be merged into nor merged away. Index every payload by surface form instead, matching what mergeAnalysisInto already accepts.
Commit the reconciled set back over the reader's choices, so a value dropped from a facet is gone rather than merely unused. Held, it returned the moment its facet did: an edit that withdrew the last row carrying a chosen value, undone, left the listing narrowed by a filter the reader had watched release, with the restored row the only one it kept.
The catalog listed what the draft records without offering any way to correct it, while the reducers that would do so had no caller. A row is a shared analysis with no token in context, so editing one is an edit to every token linked to it — the correction a mis-split word repeated a hundred times needs. Expanding a row now reveals its gloss, its morpheme breakdown and each morpheme's gloss, and the controls for merging or deleting it. Fields commit on blur and Enter and revert on Escape, so an edit is not written across every token of an analysis mid-word. Deleting is confirmed by a modal naming the concrete consequence — how many uses go blank, or which surviving homograph they fall back to. Delete ships before undo, so that copy is the only guard, and a confirmation that promised a fallback that does not exist would be worse than none. Merging is offered only to a row with pool peers, since there is otherwise nothing to reassign its tokens to, and preselects no target: it moves every use of one analysis onto another and drops the source. An edit that makes a row identical to a sibling collapses the two, which leaves the edited row gone from the listing and another's count grown. A banner above the list now names where the edit went and what the survivor counts, and the listing scrolls to it, so the pair reads as the convergence it is rather than as lost work. Nothing the write reports says a collapse happened, so the dispatch hook reads the store either side of it and follows the old links to see where they now point. Merge peers are bucketed by the normalized surface form the pool buckets by; grouping by the raw text would offer the control to homographs differing only in case and then open an empty picker. The platform-bible-react stub's Input dropped onBlur, which left every commit-on-blur field silently uncommitted in tests.
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.
This change is