🎨 Added syntax-highlighted diff view to theme editor review modal#27921
Closed
betschki wants to merge 3 commits into
Closed
🎨 Added syntax-highlighted diff view to theme editor review modal#27921betschki wants to merge 3 commits into
betschki wants to merge 3 commits into
Conversation
Contributor
WalkthroughAdds a CodeMirror-based merge diff to the theme review modal. Introduces theme-editor-languages.ts with getLanguageExtension/getLanguageLabel, adds react-codemirror-merge to dependencies and the workspace catalog, refactors theme-code-editor-modal to use the new helpers, replaces the prior preformatted modified-file preview with a keyed CodeMirrorMerge diff (lazy-loading language extensions), and adds a Playwright acceptance test verifying the merge view renders with two editor panes. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27921 +/- ##
=======================================
Coverage 73.86% 73.87%
=======================================
Files 1521 1521
Lines 128441 128441
Branches 15402 15406 +4
=======================================
+ Hits 94870 94881 +11
+ Misses 32641 32606 -35
- Partials 930 954 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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
`@apps/admin-x-settings/src/components/settings/site/theme/theme-review-modal.tsx`:
- Around line 93-104: The useEffect that loads the language extension for diffs
(checking diffPath, diffStatus, diffIsEditable) should clear any previous
extension before starting the async load and handle rejections from
getLanguageExtension to avoid unhandled promise rejections and stale state;
inside the effect call setDiffLanguageExtension(null) before invoking
getLanguageExtension(diffPath), chain a .catch handler to
setDiffLanguageExtension(null) (or another safe fallback) and log if desired,
and keep the existing cancelled flag check so the fulfilled or rejected result
does not update state after unmount/cancel.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ac2ed01-8b17-484d-a8b6-200d8c8de648
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/admin-x-settings/package.jsonapps/admin-x-settings/src/components/settings/site/theme/theme-code-editor-modal.tsxapps/admin-x-settings/src/components/settings/site/theme/theme-editor-languages.tsapps/admin-x-settings/src/components/settings/site/theme/theme-review-modal.tsxapps/admin-x-settings/test/acceptance/site/theme.test.tspnpm-workspace.yaml
- the review modal previously showed before/after as two raw <pre> blocks, leaving readers to spot differences by eye - swaps the modified-file panel for a CodeMirror merge view (react-codemirror-merge), giving line-level add/remove highlighting and synced scrolling - lazy-loads the same language extensions the editor uses, so the diff renders with matching syntax colors per file type - extracts getLanguageExtension / getLanguageLabel into theme-editor-languages.ts so the review modal can reuse them without creating a circular dep between sibling modals
- ghost/sort-imports-es6-autofix wants 'single' specifier imports before 'multiple' within the same source group; the new ./theme-editor-languages multi-name import landed between a multi-name local import and a single-name @TryGhost import, which broke that ordering - ran the rule's --fix to move the import to its correct slot
betschki
force-pushed
the
feat/theme-editor-diff-view
branch
from
ad849ad to
2a6b3fb
Compare
- the language-extension loader could leak an unhandled promise rejection when getLanguageExtension's dynamic @codemirror/lang-* import failed, and left the previous file's highlighting in place while a new file's extension loaded - clears the cached extension before the load so switching files never renders against stale highlighting, and adds a catch fallback that drops to plain text on import failure - caught by CodeRabbit review on PR TryGhost#27921
Member
|
Closing, ref #28250, the theme editor is not intended to be for developer workflows at the moment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
<pre>blocks, leaving readers to spot differences by eyeGot some code for us? Awesome 🎊!
Please take a minute to explain the change you're making:
Please check your PR against these items:
We appreciate your contribution! 🙏