Link tokens#91
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds phrase-mode and Redux phrase CRUD/selectors, new phrase hooks, arc geometry/overlays, token grouping utilities, enhanced TokenChip/TokenLinkIcon, rewrites PhraseBox/SegmentView/ContinuousView for focus-token navigation, wires Interlinearizer/Loader with phraseMode, and updates tests, config, and styles. ChangesPhrase Editing, Arcs, and Focus-Driven Views
Sequence Diagram(s)sequenceDiagram
participant UI as Interlinearizer UI
participant Store as AnalysisStore/Redux
participant Overlay as ArcOverlay
participant Utils as phrase-arc utils
UI->>Store: usePhraseLinkMap/usePhraseDispatch
UI->>Overlay: arcPaths, candidatePhraseIds, hover/focus
Overlay->>Utils: computeSplitFreeRefs(...)
Overlay->>Store: splitPhraseAtBoundary via useArcSplitHandler
Store-->>UI: updated phrase links/gloss
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
alex-rawlings-yyc
force-pushed
the
link-tokens
branch
from
99ae079 to
338010d
Compare
…om lint git workflow (it was causing a failure)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace the token-ref-keyed linear search (`[...map.values()].find(l => l.analysisId === id)`) with a new `selectPhraseLinkByAnalysisId` selector and `usePhraseLinkByIdMap` hook, giving O(1) phrase lookup by id in `ArcOverlay`, `useArcSplitHandler`, and `SegmentView`. Also: - Move the phrase-revert effect from `PhraseBox` up to `InterlinearizerInner` so it fires even when all tokens are removed from the phrase - Fix `TokenChip` state sync to use `useEffect` instead of inline mutation during render - Fix `useArcPaths` dep array to use a version counter instead of a spread to satisfy the rules of hooks - Reset `phraseMode` on project switch in `InterlinearizerLoader` - Use a stable empty map constant in `PhraseBox` to avoid breaking memo - Improve arc key to include `phraseId` and `splitAfterTokenRef`
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 4 discussions.
Reviewable status: 0 of 44 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…dow in ContinuousView, improve handling of settings in InterlinearizerLoader, add dedicated mergePhrases reducer to analysisSlice, emit data-last-token-read on all render paths
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec partially reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: 15 of 75 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
AGENTS.md line 72 at r8 (raw file):
All UI uses Tailwind CSS (via `src/tailwind.css`). Every Tailwind class is prefixed `tw:` to avoid collisions with Platform.Bible's own styles (configured in `tailwind.config.ts`). For modifier variants the prefix comes first: `tw:hover:px-3`, not `hover:tw-px-3`. Tailwind v4 at-rules (`@utility`, `@apply`, `@theme`, `@config`, `@custom-variant`, `@layer`, `@source`, `@plugin`, etc.) are **already whitelisted** in [.stylelintrc.js](.stylelintrc.js)'s `scss/at-rule-no-unknown` `ignoreAtRules` list. Do **not** flag these as Stylelint violations, and do not suggest adding them to the config or adding `stylelint-disable` comments — they already pass. If you believe a lint rule is firing, run `npm run lint` and cite the actual output rather than inferring it from the rule name.
It might be more fitting for things like this to go in a new REVIEW.md file, assuming Coderabbitai looks at that.
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 1 discussion.
Reviewable status: 15 of 75 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc and imnasnainaec).
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* Merge mostly redundant tests
* Update settings for VSCode to match our stylelint use
* Add JSDocs
* Fix pump() overwriting observerCallback via stub constructor
new ResizeObserver(() => {}) inside pump() was invoking the stub
constructor, replacing the stored observerCallback with a no-op on
every call after the first. Replace with a pre-declared stubObserver
plain object so pump() never touches the constructor again.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Sort cspell words
* Fix brit spelling now covered in AGENTS.md
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec made 1 comment.
Reviewable status: 15 of 78 files reviewed, 5 unresolved discussions (waiting on alex-rawlings-yyc).
a discussion (no related file):
🐛 Linking is misaligned when out-of-segment links are hidden:
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 3 discussions.
Reviewable status: 15 of 78 files reviewed, 1 unresolved discussion (waiting on imnasnainaec).
Previously, imnasnainaec (D. Ror.) wrote…
Would we rather the links become invisible and leave space or the gaps be closed? I'm leaning towards the gaps being closed, but it will be more work |
Previously, alex-rawlings-yyc (Alex Rawlings) wrote…
Never mind, this is a redraw timing issue. Fix is simple |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: 20 of 78 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
a discussion (no related file):
Previously, alex-rawlings-yyc (Alex Rawlings) wrote…
Never mind, this is a redraw timing issue. Fix is simple
The arcs still surf all over the place when "Hide out-of-segment link buttons" is toggled on and off. I think it's fine to keep the space there rather than close the gap.
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec partially reviewed 24 files.
Reviewable status: 44 of 78 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
2 participants
This change is
Summary by CodeRabbit
New Features
Bug Fixes