Skip to content

Rishabh060105/fix pr878 data rebuild#881

Merged
mttrbrts merged 2 commits intoaccordproject:mr-template-editorfrom
Rishabh060105:Rishabh060105/fix-pr878-data-rebuild
Apr 25, 2026
Merged

Rishabh060105/fix pr878 data rebuild#881
mttrbrts merged 2 commits intoaccordproject:mr-template-editorfrom
Rishabh060105:Rishabh060105/fix-pr878-data-rebuild

Conversation

@Rishabh060105
Copy link
Copy Markdown

Closes N/A

Fixes rich editor synchronization after externally loaded templates are rebuilt with a new modelManager.

This PR addresses a PR #878 review finding where the TipTap rich editor could parse a newly loaded sample or shared-link template before the matching modelManager was available, then ignore the later modelManager update because the markdown text itself had not changed.

Changes

  • Updated src/editors/TiptapTemplateEditor.tsx to retry parsing externally loaded template markdown when the rebuilt modelManager arrives.
  • Added tracking for pending model retries so sample and shared-link loads do not remain stuck with stale model state or a parse error.
  • Cleared pending model retries for changes originating from the rich editor itself, avoiding unnecessary external-sync retries during normal editing.
  • Updated src/store/store.ts so JSON data edits store the rebuilt HTML string and modelManager instead of assigning the full rebuild result object to agreementHtml.

Flags

  • This PR intentionally keeps the fix scoped to synchronization behavior from the PR feat(templateMark): add rich text editor #878 review findings.
  • Unit tests were run in the installed temporary PR worktree because npm ci hung in the main workspace.
  • No screenshots or video are included because this is a state synchronization fix rather than a visual UI change.

Screenshots or Video

N/A - this is a state synchronization fix. The behavior is covered by unit test verification rather than a visual UI change.

Related Issues

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

@Rishabh060105 Rishabh060105 requested a review from a team as a code owner April 16, 2026 07:11
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 16, 2026

Deploy Preview for ap-template-playground ready!

Name Link
🔨 Latest commit 75c345c
🔍 Latest deploy log https://app.netlify.com/projects/ap-template-playground/deploys/69ea3da46b050d0008848f90
😎 Deploy Preview https://deploy-preview-881--ap-template-playground.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Rishabh060105 Rishabh060105 force-pushed the Rishabh060105/fix-pr878-data-rebuild branch from 56b78a7 to f3465de Compare April 16, 2026 08:22
@Rishabh060105
Copy link
Copy Markdown
Author

Update on the latest changes:

  • Rewrote the PR branch history so all commits in the PR range include a valid DCO sign-off.
  • Fixed the data rebuild path so JSON data edits store the rebuilt html and modelManager instead of assigning the full rebuild result object to agreementHtml.
  • Updated the rich TipTap editor sync flow so externally loaded templates can reparse after the rebuilt modelManager arrives, preventing stale parse errors after sample/shared-link changes.
  • Updated editor theme handling to use isDarkMode instead of the removed backgroundColor store field.
  • Split the production Vite config from the Vitest config so Netlify builds no longer import vitest/config, fixing the ERR_REQUIRE_ESM build failure.

Validation completed locally:

  • npm run build passes.
  • npm run test:unit passes with 25 test files and 197 tests.
  • DCO and Netlify deploy checks are now passing on GitHub.

The only remaining blockers shown are maintainer/code-owner review and workflow approvals for the required GitHub Actions checks.

@Rishabh060105
Copy link
Copy Markdown
Author

@dselman could you you go through this PR and suggest if any changes are needed?

@mttrbrts
Copy link
Copy Markdown
Member

@dselman could you you go through this PR and suggest if any changes are needed?

Thanks for all the fixes and enhancements @Rishabh060105 . Please have your PR target the branch mr-template-editor so that it's clear what changes you have made

@Rishabh060105 Rishabh060105 force-pushed the Rishabh060105/fix-pr878-data-rebuild branch from 459a4e8 to 75c345c Compare April 23, 2026 15:41
@Rishabh060105 Rishabh060105 changed the base branch from main to mr-template-editor April 23, 2026 15:51
@Rishabh060105
Copy link
Copy Markdown
Author

Rishabh060105 commented Apr 23, 2026

@mttrbrts I’ve now retargeted this PR to mr-template-editor so the diff only shows the follow-up fixes on top of the original TipTap editor branch.

The remaining changes in this PR are scoped to:

  • rich editor/modelManager resync in TiptapTemplateEditor.tsx
  • rebuild result handling in store.ts
  • Vite/Vitest config split for CI

DCO is now passing on the latest head. If needed, the remaining checks may need to be re-triggered/approved against the updated PR base and head.

Retry TipTap template parsing when external sample or shared-link loads receive the rebuilt modelManager, so the rich editor does not get stuck on stale model state.

Also store rebuilt html/modelManager after JSON data edits and split Vitest config from vite.config.ts so CI can load the production Vite config cleanly.

Signed-off-by: Rishabh060105 <rishabhj2005@email.com>
@Rishabh060105 Rishabh060105 force-pushed the Rishabh060105/fix-pr878-data-rebuild branch from 75c345c to 8f4daa3 Compare April 23, 2026 15:57
@mttrbrts mttrbrts requested a review from Copilot April 23, 2026 21:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a synchronization race between externally loaded templates (samples/share links) and the TipTap rich editor by ensuring the editor reparses markdown when the rebuilt modelManager arrives, and it corrects store state updates so agreementHtml remains a string after data-driven rebuilds.

Changes:

  • Add a pending “retry parse when modelManager updates” mechanism in TiptapTemplateEditor for external template loads.
  • Clear pending retries for edits originating from the rich editor to avoid unnecessary re-parses during normal editing.
  • Fix setData to store { html, modelManager } outputs correctly instead of assigning the whole rebuild result object into agreementHtml.
  • Split Vitest config out of vite.config.ts into vitest.config.ts.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
vitest.config.ts Introduces a dedicated Vitest config merged with the Vite config.
vite.config.ts Removes embedded Vitest configuration; exports only Vite build/dev config.
src/store/store.ts Fixes setData rebuild state assignment to keep agreementHtml as a string and update modelManager.
src/editors/TiptapTemplateEditor.tsx Adds modelManager-aware reparse retry logic to resolve external-load synchronization issues.

Comment thread src/editors/TiptapTemplateEditor.tsx
Comment thread src/store/store.ts
Signed-off-by: Rishabh060105 <rishabhj2005@email.com>
@Rishabh060105
Copy link
Copy Markdown
Author

Hi @mttrbrts, I addressed the Copilot feedback with focused regression tests for both fixes in this PR.

Added:

  • src/tests/components/TiptapTemplateEditor.test.tsx to cover the external-load/modelManager sync case
  • src/tests/store/setData.test.tsx to cover the setData() rebuild result handling

These verify:

  • the editor reparses when modelManager changes without another editorValue change
  • agreementHtml remains a string after setData()
  • modelManager is updated from the rebuild result

Validation run:

  • targeted Vitest runs for both new tests
  • full npm run test:unit

Copy link
Copy Markdown
Member

@mttrbrts mttrbrts left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @Rishabh060105 !

@mttrbrts mttrbrts merged commit c6dfd77 into accordproject:mr-template-editor Apr 25, 2026
2 checks passed
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.

3 participants