Skip to content

Implement a variety of small refactors - #291

Merged
Avni2000 merged 13 commits into
mainfrom
simplification
Jul 24, 2026
Merged

Implement a variety of small refactors#291
Avni2000 merged 13 commits into
mainfrom
simplification

Conversation

@Avni2000

@Avni2000 Avni2000 commented Jul 23, 2026

Copy link
Copy Markdown
Owner

This PR attempts to reuse components more, and deduplicate code as much as possible. This does not touch core functionality

Summary by CodeRabbit

  • New Features
    • Added a reusable warning modal for actions that may overwrite active edits.
    • Improved conflict-resolution controls with consistent confirmation, Undo/Redo, and “take all” button behavior.
  • Improvements
    • Upgraded cell rendering to use a unified source renderer for code/Markdown, with refined static syntax highlighting and diff support.
    • Optimized large conflict view rendering so off-screen rows upgrade to near-viewport on first intersection.
  • Bug Fixes
    • Improved resolver history navigation for undo/redo and revision jumps, ensuring correct state restoration.

Avni2000 added 12 commits June 11, 2026 20:18
- Merge StaticHighlightedCode and StaticDiffContent into one CellSource
  component (they differed only in whether diff marks were computed)
- Remove the markdown fence highlight machinery (async per-language loading
  cache and the duplicated DOM-builder render path that existed only for it);
  markdown code fences now render unhighlighted
- Replace the 32-entry rendermime registry LRU (plus eviction/disposal) with
  one lazy registry: session credentials come from window.location and never
  change within a page
- Stop threading the registry through CellOutputs/RenderMimeOutput props
…ght upgrade

- Drop the markdown <textarea> editor branch; CodeMirror handles all cell
  types (markdown simply gets no language extension), removing a duplicated
  blur-autosave handler and the textarea-specific styles
- Extract the shared blur handler and the twice-duplicated reorder indicator
- Make the off-viewport lightweight mode upgrade-only: rows never swap back
  to plain text, so scrolling can't replace DOM under an in-progress text
  selection
- Inline the trivial renderViewportModal wrapper
- Remove the now-dead has-syntax-highlight style selector
…d stale deps

highlightCode walks the tree linearly and computeDiffMarks builds ranges
front-to-back, so both inputs to buildLineSegments are already in document
order. Skipping unstyled tokens also lets adjacent plain text collapse into
a single segment instead of one span boundary per lexer token.
@coderabbitai

coderabbitai Bot commented Jul 23, 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: fa6e8c04-44d3-49f4-81d8-a0104971e692

📥 Commits

Reviewing files that changed from the base of the PR and between fbde114 and 635ff12.

📒 Files selected for processing (1)
  • packages/web/client/src/components/WarningModal.tsx

📝 Walkthrough

Walkthrough

The PR consolidates static cell rendering, introduces shared warning and guarded-action flows, refactors conflict-row controls and editing behavior, centralizes history navigation, and changes render-mime registry creation to a session-scoped singleton.

Changes

Cell rendering and output integration

Layer / File(s) Summary
Static cell rendering
packages/web/client/src/components/CellContent.tsx
CellSource handles static markdown, code, and diff rendering; ordered syntax tokens and inline ranges are consumed directly, while MarkdownContent drops fenced-code enhancement and its theme prop.
Cell and resolved-row rendering
packages/web/client/src/components/CellContent.tsx, packages/web/client/src/components/MergeRow.tsx, packages/web/client/src/styles.ts
Cell and resolved-row views use the updated renderers, resolved editing uses CodeMirror, and lightweight rows upgrade once after entering the viewport.
Render-mime registry lifecycle
packages/web/client/src/components/CellContent.tsx
Outputs obtain a module-level render-mime registry singleton instead of receiving a cached registry prop.

Conflict-resolution interactions

Layer / File(s) Summary
Warning dialogs and guarded actions
packages/web/client/src/components/WarningModal.tsx, packages/web/client/src/components/ConflictResolver.tsx, packages/web/client/src/components/MergeRow.tsx
Warning dialogs use WarningModal, and guarded actions coordinate edit prompts, confirmation, focus handling, and duplicate-click suppression.
Conflict controls and history state
packages/web/client/src/components/ConflictResolver.tsx, packages/web/client/src/store/resolverStore.ts, packages/web/client/src/styles.ts, packages/web/client/src/components/MergeRow.tsx
Undo/Redo, history, take-all controls, conflict columns, reorder indicators, and snapshot restoration use shared handlers, mapped descriptors, centralized store logic, and new button styling.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConflictResolver
  participant WarningModal
  participant resolverStore
  participant MergeRow
  ConflictResolver->>WarningModal: request confirmation for guarded action
  WarningModal-->>ConflictResolver: confirm or cancel
  ConflictResolver->>resolverStore: apply resolution or navigate history
  resolverStore-->>MergeRow: update conflict and resolution state
  MergeRow-->>ConflictResolver: render updated row controls
Loading

Possibly related PRs

Poem

A rabbit hops through cells of code,
With diff marks bright along the road.
“Confirm?” asks Modal, soft and clear,
Undo and redo now draw near.
Snapshots leap, conflicts fade—
A tidy merge the burrow made.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is too vague and generic to convey the main change in the PR. Use a concise title that names the primary refactor, such as introducing reusable warning modal and cell rendering components.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch simplification

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 `@packages/web/client/src/components/WarningModal.tsx`:
- Around line 36-58: Update the WarningModal component to behave as an
accessible modal dialog: add alertdialog semantics with aria-modal, focus the
dialog or confirm button on mount, trap Tab navigation within the modal actions,
restore the previously focused element on unmount, and invoke onCancel for
Escape. Keep the existing confirmation and cancellation behavior unchanged.
🪄 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: dec5ae20-ff56-4353-ae41-b4fd08e9b062

📥 Commits

Reviewing files that changed from the base of the PR and between 53e70bb and fbde114.

📒 Files selected for processing (6)
  • packages/web/client/src/components/CellContent.tsx
  • packages/web/client/src/components/ConflictResolver.tsx
  • packages/web/client/src/components/MergeRow.tsx
  • packages/web/client/src/components/WarningModal.tsx
  • packages/web/client/src/store/resolverStore.ts
  • packages/web/client/src/styles.ts

Comment thread packages/web/client/src/components/WarningModal.tsx
@Avni2000
Avni2000 enabled auto-merge July 23, 2026 18:03
@Avni2000
Avni2000 disabled auto-merge July 23, 2026 18:03
@sonarqubecloud

Copy link
Copy Markdown

@Avni2000
Avni2000 merged commit a6500e2 into main Jul 24, 2026
8 checks passed
@Avni2000
Avni2000 deleted the simplification branch July 24, 2026 02:54
@coderabbitai coderabbitai Bot mentioned this pull request Jul 26, 2026
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