Skip to content

docs(checks): proposal — highlight repeated words in the verse editor#328

Closed
JEdward7777 wants to merge 2 commits into
mainfrom
jel-highlight-in-verse-proposal
Closed

docs(checks): proposal — highlight repeated words in the verse editor#328
JEdward7777 wants to merge 2 commits into
mainfrom
jel-highlight-in-verse-proposal

Conversation

@JEdward7777

@JEdward7777 JEdward7777 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Docs-only proposal responding to feedback from @joelthe1 and Ulf (raised during the #305 review): add an in-editor highlight/underline as a second visual for the repeated-word check, so flagged words are visible in the verse text itself and not only in the Checks panel.

The proposal exists because in-text coloring is not a small change — the drafting target column is a plain <textarea>, which cannot color individual words, and there is no rich-text editor dependency in the app today. Choosing how to enable per-word decoration is an editor-foundation decision the team should make together, so this goes out as a proposal rather than a unilateral implementation.

📄 Read the full doc: docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md

What it covers

  • The original request (Joel/Ulf) and the intended UX: only active repeated words turn red; ignored words render normal; no other change to editing.
  • The existing check pipeline it would build on (useRepeatedWordsCheckuseResolvedFindingsChecksPanel).
  • The core <textarea> constraint and the caret/IME/spellcheck/focus behaviors that must be preserved.
  • Why translation software makes this harder: UTF-16 vs. grapheme clusters, combining marks / complex shaping, RTL & bidi, variable font metrics.
  • Four options with honest complexity/danger analysis: (A) panel-only / do nothing now, (B) CSS overlay mirror, (C) raw contenteditable (not recommended), (D) real editor engine + decorations — CodeMirror 6 preferred, behind a feature flag (recommended).
  • A note that a full, robust solution will eventually be needed given the scripts and glyphs this product must support.

Ask

Reviewers: please pick a direction (Q1–Q5 at the end of the doc). No code changes are included; this PR is the doc only.

Relates to #277, #278

Summary by CodeRabbit

  • Documentation
    • Added a design proposal for highlighting repeated-word findings directly inside the editable verse.
    • Compares multiple in-editor rendering approaches and recommends a rich-editor direction for accurate visual decorations.
    • Covers implementation requirements including grapheme- and Unicode-consistent text offset mapping, updating on existing panel-driven save behavior, optional read-only “view” highlighting, and rollout behind a feature flag.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e07f785e-5c5e-48d5-bc2b-c8779b63e0ad

📥 Commits

Reviewing files that changed from the base of the PR and between edffb3e and b3d0cc0.

📒 Files selected for processing (1)
  • docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md

📝 Walkthrough

Walkthrough

This PR adds a new proposal document for highlighting repeated-word findings inside the verse editor. It covers the textarea constraint, four implementation options, shared requirements, and open decision questions. No code changes are included.

Changes

Proposal Documentation

Layer / File(s) Summary
Repeated-word highlight design proposal
docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md
New document detailing the <textarea> highlighting constraint, four implementation options (panel-only, CSS overlay, contenteditable, real editor engine—recommended), cross-cutting requirements, and placeholders for decisions and out-of-scope items.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: kaseywright

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the docs-only proposal to highlight repeated words in the verse editor.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jel-highlight-in-verse-proposal

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md`:
- Around line 247-250: The `/view` highlight note in the HL5 section overstates
that read-only coloring is independent of the earlier mapping work; update the
wording to make clear that the `start_position` to rendered-range mapping and
grapheme/NFC handling are still shared with the editable path. Adjust the
sentence in the HL5 bullet so it explicitly says `/view` highlighting is simpler
visually, but still depends on the same range-mapping implementation described
above, using the HL5 label and the repeated-word-check proposal text as the
anchor.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d2cc60d1-c763-47dc-9f78-f2c37f2b0dcf

📥 Commits

Reviewing files that changed from the base of the PR and between 46cf442 and 0594a31.

📒 Files selected for processing (1)
  • docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md

Comment thread docs/proposals/repeated-word-check/highlight-in-verse-suggestion.md Outdated
@JEdward7777
JEdward7777 force-pushed the jel-highlight-in-verse-proposal branch from edffb3e to b3d0cc0 Compare July 7, 2026 13:10
@JEdward7777

Copy link
Copy Markdown
Contributor Author

Had a conversation with Joel, where he indicated that the highlighting he had requested is not in the editor pane but in the repeated word check cards themselves. Therefore this suggestion document needs to be updated before additional review is needed.

@JEdward7777

Copy link
Copy Markdown
Contributor Author

Superseded by #345. This proposal analyzed highlighting the repeated word in the editor pane, which would have required moving the drafting surface to a rich-text/overlay editor. Talking it through in person, the actual ask was the much simpler in-card highlight — each check card now shows the verse text windowed around the match with the repeated word emphasized in place. Given the small scope, the solution is proposed via the implementation itself in #345 rather than a separate proposal round. Keeping this PR's editor-surface analysis as historical context in case in-editor highlighting is ever wanted.

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.

1 participant