Skip to content

Make the morpheme-break reset explicit: rename Delete, and reset on the bare surface form - #173

Open
alex-rawlings-yyc wants to merge 3 commits into
mainfrom
improve-morph-break-ui
Open

Make the morpheme-break reset explicit: rename Delete, and reset on the bare surface form#173
alex-rawlings-yyc wants to merge 3 commits into
mainfrom
improve-morph-break-ui

Conversation

@alex-rawlings-yyc

@alex-rawlings-yyc alex-rawlings-yyc commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What & why

Two usability problems with the morpheme-break popover's destructive control:

  1. "Delete" is alarming and imprecise. The button doesn't delete the token or its gloss — it returns the word to its unsegmented state.
  2. The save gate blocked the wrong thing. Typing the bare surface form (unbelievable over un- believe -able) disabled Done and silently dismissed. But asking for "one morpheme equal to the whole word" is a request for the unsegmented state, so it should reset rather than be rejected.

Changes

Renamed the button to "Reset" (value only — the %interlinearizer_morphemeEditor_delete% key is unchanged, see Open question below). The onDelete prop is renamed onReset.

Reworked the commit gate. isMeaningless is retired; committing now resolves to one of four outcomes:

Draft state Done Enter / outside-click
Empty disabled + format hint no-op
Unchanged, breakdown exists enabled → dismiss dismiss
Unchanged, no breakdown enabled → save save
== surface form enabled → reset reset
Anything else enabled → save save

Notable points:

  • Done is no longer disabled while unchanged. The panel always opens pre-filled, so disabling it would leave a dead primary button on every open with no explanation of why. Done means "I'm finished here"; empty is the only true error state, and it now gets a hint explaining the expected format.
  • An unedited draft saves when no breakdown exists yet. A caller can pre-fill a suggested segmentation; accepting it as-is is new information, not a rewrite. Only an unedited draft over an existing breakdown dismisses.
  • A single morpheme differing from the surface text still saves (e.g. runningrun). Only a form equal to the surface text means "no segmentation". Verified safe: MorphemeAnalysis.form is documented as the form "as it appears in this analysis", morphemes carry no offsets, and — unlike the token-level Segment.baselineText.slice(charStart, charEnd) === Token.surfaceText — there is no invariant requiring morphemes to reconstruct the surface.

Added a confirmation before an irreversible reset. Both routes (the button and typing the bare surface form) funnel through one requestReset(), which confirms only when the reset would destroy gloss data this token solely owns.

The second half of that condition matters: deleteMorphemes already branches on whether the payload is shared, forking a private clone so co-linked tokens keep their morphemes. When the payload is shared, nothing is lost project-wide — the user is detaching one occurrence — so no prompt. A breakdown with no glosses is bare segmentation that's cheap to retype, so no prompt there either. The friction lands only where the loss is real, which keeps experimenting-with-splits a single click.

Justified because the app has no undo, and the repo already confirms a smaller loss (UnlinkPhraseConfirm).

The confirmation replaces the popover panel's own content rather than opening a second surface. The panel is portaled to document.body, so it floats over the token chip and can't reflow it, and nesting a modal inside this already-modal popover would stack two focus traps. An outside click while confirming dismisses without resetting — the confirmation exists precisely because the loss is irreversible, so a stray click must not answer it.

Wiring. New selectMorphemeResetLosesGlosses selector + useMorphemeResetLosesGlosses hook; TokenChip threads the result to the popover as needsResetConfirm. The selector reads sharing through the existing memoized selectApprovedTokenCountByAnalysisId — the same approved-link count the reducer's isPayloadSharedByOtherLinks tests — so the two can't disagree about what "shared" means.

Strings. Three new keys in localizedStrings.json (_emptyHint%, _confirmResetPrompt%, _confirmResetAction%), all localized.

The confirm copy is deliberately count-free (Discard this breakdown and its glosses? rather than Discard 2 morpheme glosses?). This codebase's localization is a flat key→string lookup with a single manual .replace() call site and no plural-shaped keys, so a counted string would mean hand-rolling the project's first pluralization scheme in English's two-form model — which misfits languages with dual forms, last-digit selection, or no count inflection at all. The user can see the morphemes in the chip behind the panel anyway.

Relationship to #168

#168 migrates this file's three buttons to platform-bible-react Button. This PR deliberately keeps raw <button>, because writing pbr components here would conflict with #168 rather than rebase onto it. The new confirm buttons reuse the exact class strings #168 maps to variant/size, so its migration extends to them mechanically.

Post-merge follow-up: convert the new buttons alongside the rest, and adopt PopoverDescription/PopoverTitle for the confirm copy.

Open question

"Reset" vs. "Remove breakdown" is still under review with the team. The English value changed but the _delete% key did not, so flipping the wording is a one-line diff rather than churn across translation files and POPOVER_STRING_KEYS. _confirmResetAction% is a separate key so a translator can use a shorter verb in the confirmation, where the prompt has already established the subject.

Testing

npm test — 1594 passing, 100% coverage (statements / branches / functions / lines). npm run lint clean.

New coverage: the reset-on-surface-form path, the no-breakdown dismissal, single-morpheme saves, the empty hint, all six confirmation behaviors (both entry routes, accept, cancel, outside-click, and the no-confirm-needed case), the four selector cases, and prop threading through TokenChip.

Two pre-existing tests asserting that a whole-word draft is blocked were rewritten — that behavior is intentionally replaced by the reset.
`


This change is Reviewable

Summary by CodeRabbit

  • New Features

    • Replaced morpheme “Delete” actions with a “Reset” workflow, including an empty-draft hint.
    • Added a reset confirmation prompt with Cancel/Confirm, and safe outside-click behavior while confirmation is pending.
    • “Done” is now disabled when the morpheme draft is empty, and unchanged drafts dismiss without rewriting.
  • Bug Fixes

    • Prevents loss of shared glosses by only requiring confirmation when a reset would discard gloss data that isn’t preserved elsewhere.

@alex-rawlings-yyc alex-rawlings-yyc self-assigned this Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Changes

Morpheme reset workflow

Layer / File(s) Summary
Gloss-loss detection
src/store/analysisSlice.ts, src/__tests__/store/analysisSlice.test.ts
Adds and tests a selector that detects whether resetting a token would discard unshared morpheme glosses.
Reset state wiring
src/components/AnalysisStore.tsx, src/components/TokenChip.tsx, src/components/__mocks__/AnalysisStore.tsx, src/__tests__/components/TokenChip.test.tsx
Passes reset callbacks and gloss-loss confirmation state from TokenChip to MorphemeBreakdownPopover, with updated mock and component coverage.
Editor reset and commit behavior
src/components/MorphemeEditor.tsx, contributions/localizedStrings.json
Replaces onDelete with onReset, adds reset confirmation and empty-draft handling, and updates localized labels and prompts.
Editor interaction coverage
src/__tests__/components/MorphemeEditor.test.tsx
Tests reset visibility, saving, empty hints, confirmation, cancellation, outside interactions, and confirmation-disabled behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TokenChip
  participant AnalysisStore
  participant analysisSlice
  participant MorphemeBreakdownPopover
  participant Editor
  TokenChip->>AnalysisStore: Read reset gloss-loss state
  AnalysisStore->>analysisSlice: Select state for token
  analysisSlice-->>AnalysisStore: Return confirmation flag
  AnalysisStore-->>TokenChip: Provide resetLosesGlosses
  TokenChip->>MorphemeBreakdownPopover: Pass onReset and needsResetConfirm
  Editor->>MorphemeBreakdownPopover: Choose Reset
  alt Confirmation required
    MorphemeBreakdownPopover-->>Editor: Render confirmation prompt
    Editor->>MorphemeBreakdownPopover: Confirm reset
  end
  MorphemeBreakdownPopover->>TokenChip: Invoke onReset
  TokenChip->>TokenChip: Dispatch morpheme deletion
Loading

Suggested reviewers: imnasnainaec

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title captures the main shift from delete semantics to explicit reset behavior and the bare-surface-form reset path.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch improve-morph-break-ui

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@alex-rawlings-yyc alex-rawlings-yyc linked an issue Jul 27, 2026 that may be closed by this pull request
coderabbitai[bot]

This comment was marked as outdated.

@alex-rawlings-yyc

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

alex-rawlings-yyc and others added 3 commits July 27, 2026 16:00
handleSave returns early on an empty draft without closing, so an
outside
click after clearing the input left the modal popover stuck open.
PR #168 migrated this popover's buttons to platform-bible-react `Button`
but left the breakdown text field and its label as raw HTML. Bring them
in line with the convention the three project modals already follow, so
the field picks up the host's focus ring, disabled styling, and dark-mode
tokens instead of the hand-rolled approximations.

Drops the classes `Input` already supplies (rounded, border, border-input,
px/py, text-sm) and keeps only `w-full` and the `font-mono` that the
morpheme forms want. `Label` defaults to text-sm, so the popover's
smaller `text-xs` stays as an explicit override.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve morph-break ui

1 participant