PT-4020 Backspace confirmation

[jolierabideau]

Screen.Recording.2026-07-01.at.12.21.20.PM.mov

Code Review Summary

Branch: pt-4020-backspace-confirmation

Base: origin/main

Date: 2026-06-30

Review model: Claude Opus 4.8

Files changed: 1 (original branch) → 2 (after in-review changes)

Overview

This PR adds the styling half of a two-step intentional-delete affordance for verse markers in the scripture editor. The goal is to prevent verse markers from being deleted too easily via the keyboard: the first Backspace/Delete "arms" the marker (selects it and gives it destructive styling), and a second Backspace/Delete actually removes it.

The deletion logic lives in a coordinated upstream PR in the external @eten-tech-foundation/platform-editor package (~/dev/scripture-editors, branch pt-4020-backspace-confirmation), where ImmutableVerseNode applies a .verse-selected class on the armed state. This paranext-core PR supplies the theme-aware styling for that class, overriding the package's default hardcoded #dc2626 rule with the platform's --destructive theme token — exactly as the upstream package's own comment directs ("see paranext-core's _usj-nodes.scss").

During review the styling was refined to match shadcn's destructive affordance (10% tint in light themes, 20% under .dark), switched to the repo's standard color-mix(in oklab, …) color space, and propagated to the Storybook demo copy.

API Changes

None. The only changed files are CSS/SCSS stylesheets. No exports, type signatures, command names, or @papi/ surfaces were added, modified, or removed.

Findings

Critical — Must address before merge

None.

Important — Should address before merge

• Rule placed in _usj-nodes.scss (the verbatim "Copied from upstream" file) rather than the dedicated _editor-overrides.scss. (Dismissed — placement is correct and matches the established convention. .verse-selected is a per-node USJ style maintained upstream in usj-nodes.css alongside .verse/.chapter; the file already hosts theme-token deviations from upstream (16 var(--…) usages, e.g. --scripture-accent, --muted-foreground); upstream's own comment explicitly points the --destructive override to _usj-nodes.scss; and _editor-overrides.scss is reserved for editor chrome — .editor-container, .toolbar, ::selection, focus rings — not per-node USJ styles.)
• Dark-mode contrast: full-strength color: var(--destructive) text on a 15% tint of the same dark-red token risked low contrast in dark themes (fixed during review: matched shadcn's destructive button affordance — color-mix(… --destructive 10%) in light themes, bumped to 20% under .dark .verse-selected, with a comment citing the button variant it mirrors.)
• Storybook demo copy lib/platform-bible-react/src/components/demo/scripture-editor/usj-nodes.css did not contain .verse-selected, so the demo would not reflect the armed-for-delete styling (fixed during review: added the equivalent rule with a var(--destructive, #dc2626) fallback so it renders even where the theme token is absent, plus the matching .dark split.)

[Author response: Finding 1 dismissed with a strong rationale grounded in the upstream repo's conventions (verified against ~/dev/scripture-editors). Findings 2 and 3 were fixed in-review at the author's direction.]

Minor — Consider

• Comment-style nit: the file mixes /* */ and // comments. (Moot — was only relevant if the rule moved to _editor-overrides.scss, which uses //. The rule correctly stayed in _usj-nodes.scss, which uses /* */.)
• Semantic color-pairing: design guide nudges toward background + -foreground pairing rather than same-token-on-same-token. (Resolved — the rule now matches shadcn's own vetted destructive component pattern exactly.)
• color-mix(in srgb, …) mixed an OKLCH-authored token in sRGB, which can look muddier than in oklab (fixed during review: switched both files to color-mix(in oklab, …), matching the repo's dominant convention — 15 existing in oklab usages vs the 4 new in srgb ones, including the nearest precedent in the editor's own editor-overrides.css.)
• Third sibling copy extensions/src/platform-enhanced-resources/src/_usj-nodes.scss lacks the rule. (Dismissed — it is a partial/divergent copy, and enhanced-resources is a read-only resource viewer where verse deletion cannot occur, so the rule is intentionally out of scope.)

[Author response: Author confirmed the dispositions; the one actionable item (oklab) was fixed in-review.]

Template Propagation

Shared Regions Modified

None. The changed files contain no #region shared with … markers.

Extension Config Changes

• [n/a] extensions/src/platform-scripture-editor/src/_usj-nodes.scss — extension source stylesheet, not a config/build file. No template propagation required.
• [n/a] lib/platform-bible-react/src/components/demo/scripture-editor/usj-nodes.css — Storybook demo asset, not a config/build file. No template propagation required.

Positive Observations

• Uses the --destructive theme token instead of the package's hardcoded #dc2626, so the armed state is theme- and dark-mode-aware — exactly the override the upstream package documents and requests.
• Tightly scoped to styling; the behavior is correctly delegated to the external editor package via a coordinated upstream PR.
• The rule is well-commented, explaining both the two-step UX and (after review) the shadcn affordance it mirrors.
• Placement matches the upstream node-style convention rather than inventing a new home for the rule.

Interview Notes

• Stated purpose: Add an intentional two-step deletion for verse markers to prevent too-easy keyboard deletion — first Backspace/Delete selects + styles the marker, second deletes it. This PR is the styling half only.
• Coordinated dependency: The behavior is implemented in an upstream editor PR — ~/dev/scripture-editors, branch pt-4020-backspace-confirmation — where ImmutableVerseNode applies the .verse-selected class (latest commit guards ImmutableVerseNode.isSelected against a Lexical element-point throw). The styling and behavior need to land together.
• Author understanding: Strong. The author explained the upstream "correlating file" convention (why the rule belongs in _usj-nodes.scss, not the overrides file) and that the destructive theming was copied from the shadcn theme — both of which checked out against the upstream repo and lib/platform-bible-react. No areas were deferred to AI; no uncertainty expressed.

In-Review Quality Check

All checks pass after the in-review changes:

• npm run typecheck — passed cleanly
• npm run lint — passed (0 errors; 2 pre-existing no-console warnings in an unrelated TS hook, untouched)
• Prettier (changed files only) — already clean, no changes
• npm test — passed (8 files, 191 tests)

Note: the required build:platform-bible-react step regenerated lib/platform-bible-react/dist/ artifacts, but those were a 10,852-line minifier-identifier-churn diff containing zero verse-selected references (the demo CSS is not bundled into dist). They were unrelated build drift from a differently-built committed dist, so they were reverted and excluded from this PR.

Suggested Review Focus

Prioritized areas for the author-reviewer meeting:

• Coordinated landing: confirm the @eten-tech-foundation/platform-editor version that ships the delete behavior (and applies .verse-selected) is the one this repo pins/installs — package.json requests ~0.8.14, installed is 0.8.15. Ensure the styling PR and the editor-behavior PR merge together so neither ships half a feature.
• Dark-mode legibility: visually verify the armed state across all four theme/appearance combinations (Platform/Paratext × light/dark) now that it matches shadcn's /10→/20 split.
• Upstream sync: when syncing, mirror the 10%/20% light/dark split into upstream usj-nodes.css (using #dc2626) so the two copies stay aligned.

AI-assisted — session

---

This change is 

[katherinejensen00]

I like how clear the colors and highlighting are. Great work, Jolie! One note, when this and the related Editor PR pass, Claude suggested checking the following scenario: When the upstream class-application lands, worth verifying ordinary text selection isn't painted destructive-red. Per the block comment, only the blink is gated on .verse-delete-armed — the destructive background + red text apply to any verse marker inside a normal range selection. That's likely fine, but once the behavior is live it'd be good to eyeball that selecting a paragraph doesn't make every contained verse marker read as "armed for deletion."

@katherinejensen00 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved.

[katherinejensen00]

Great additions! Just a couple of small things to consider and you are all set. I went ahead and approved the changes since the suggestions are so small

@katherinejensen00 reviewed 6 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on jolierabideau).

---

extensions/src/platform-scripture-editor/src/verse-delete-tooltip/verse-delete-tooltip-overlay.component.tsx line 125 at r2 (raw file):

            }}
          />
          <TooltipContent side="bottom" align="start" showArrow={false}>

NIT Claude Suggestion that seemed like a good idea. I didn't know you could do something like this.

This hint gates a destructive action but is visual-only, so screen-reader users get no warning before deleting a verse marker. The trigger is aria-hidden="true" / tabIndex={-1} and the "press again" copy lives entirely in TooltipContent, so nothing is announced when the delete is armed. This pattern was fine to copy from ParagraphMarkerTooltipOverlay (a passive hover hint), but a destructive-action confirmation has a higher a11y bar. Consider mirroring the armed state into an aria-live="polite" (or assertive) region so the "Backspace again to delete selection" message is spoken when it arms.

---

extensions/src/platform-scripture-editor/src/verse-delete-tooltip/verse-delete-tooltip-overlay.component.tsx line 128 at r2 (raw file):

            {armed ? (
              <>
                <kbd>{confirmingKey(armed.intent)}</kbd> {message}

NIT Claude Finding

Nice improvement localizing the copy — one gap: the key label stays English and the word order is hard-coded, so Spanish reads "Backspace de nuevo para eliminar la selección." The <kbd> value comes from confirmingKey (always English "Backspace"/"Delete") and the JSX bakes in key-first ordering that not every language will want. If that's a deliberate call to leave physical key names untranslated, all good — otherwise consider a single localized string with a {key} placeholder so translators own both the word and its position.