Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting bar: Add support for CodeMirror 6 #65

Merged

Conversation

personalizedrefrigerator
Copy link
Contributor

@personalizedrefrigerator personalizedrefrigerator commented Feb 28, 2024

Summary

This pull request builds on #64 to allow the formatting bar to work with CodeMirror 6.

Notes

  • This pull request disables some functionality that was causing errors.
  • The CodeMirror 6 formatting bar uses CodeMirror's built-in tooltip functionality.
    • Currently, the formatting bar is left-aligned with the highlighted content in CodeMirror 6. This should be possible to fix with the Tooltip.offset property, but will require extra work to ensure that the formatting bar does not go offscreen.
  • This pull request tries to avoid rebasing (as I've heard this can break in-progress code reviews). As such, it includes a few "revert changes to..." commits. On request, I can squash some of this pull request's commits into more meaningful commits.

Testing

  1. Enable the beta editor
  2. Select text
  3. Toggle bold formatting using the toolbar (on, then off)
  4. Toggle italic formatting (on, then off)
  5. Toggle code (on, then off)
  6. Add a link to the selection
  7. Select different text
  8. Toggle all 7 colors
  9. Repeat steps 2-8 for the legacy editor

Additional testing (done later):

  1. Enable Quick Commands
  2. Verify that the formatting bar can still be used in the CodeMirror 6-based editor
  3. Switch to CodeMirror 5
  4. Verify that Quick Commands can still be used

Screen recording

enhancement-formatting-bar.mp4

@personalizedrefrigerator personalizedrefrigerator marked this pull request as draft February 28, 2024 23:27
@personalizedrefrigerator
Copy link
Contributor Author

personalizedrefrigerator commented Feb 28, 2024

In some cases, the color selection buttons don't seem to work. I'm converting this pull request to a draft until that is resolved.

Edit: This was caused by a bug in the beta editor's CodeMirror 5 emulation. This pull request works around it by using the Joplin-provided API for registering commands in the beta editor.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review February 29, 2024 03:53
Comment on lines +24 to +28
// FIXME(for:cm6): Disabled because the following logic is broken in
// the CM6 editor.
if (cm.cm6) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now (on CodeMirror 6), I've disabled several parts of the plugin that fail to load in CodeMirror 6.

The Quick Commands extension uses functionality not available through Joplin's CodeMirror
5 emulation. Thus, it will be disabled in Joplin's CodeMirror 6-based beta editor until
it can be ported separately.
@SeptemberHX
Copy link
Owner

Thank you for the PR! I tested the branch and found that all features of the plugin are not working under Joplin version 2.14.16, and the following error appeared in the developer tools:

Logger.ts:271 14:04:53: loadContentScripts: Cannot find module '@codemirror/state'
Require stack:
- /Users/septemberhx/.config/joplin-desktop/cache/com.septemberhx.Joplin.Enhancement/driver/codemirror/index.js
- /Applications/Joplin.app/Contents/Resources/app.asar/node_modules/@joplin/lib/shim-init-node.js
- /Applications/Joplin.app/Contents/Resources/app.asar/node_modules/@joplin/lib/BaseApplication.js
- /Applications/Joplin.app/Contents/Resources/app.asar/app.js
- /Applications/Joplin.app/Contents/Resources/app.asar/index.html

Could it be that an error occurred during the process of running npm install; npm run dist?

@personalizedrefrigerator
Copy link
Contributor Author

I've made the CodeMirror 6 requires dynamic rather than static (see commit).

I suspect that import ... from '@codemirror/...' was failing in CodeMirror 5 -- the beta markdown editor provides a custom require function to plugins that allows importing Joplin's versions of the @codemirror packages. This is not provided in CodeMirror 5.

Interestingly, the import ... from '@codemirror/' imports don't seem to have been failing in CodeMirror 5 on my system. Because running require('@codemirror/state') in the Joplin development tools does fail, I suspect that it's working because of how Webpack optimized the built code.

@SeptemberHX SeptemberHX merged commit bdf2b28 into SeptemberHX:master Mar 2, 2024
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.

2 participants