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

#701 Improve note support : WYSIWYG markdown #715

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Tukks
Copy link

@Tukks Tukks commented Dec 3, 2024

First implementation with a wysiwyg markdown editor. Update:

  • Add Lexical markdown editor
  • consistent rendering between card and preview
  • removed edit modal, replaced by preview with save action
  • simple markdown shortcut: underline, bold, italic etc...

@Tukks Tukks force-pushed the feat/fulle-featured-markdown-editor branch 3 times, most recently from 08c27a7 to 64120fc Compare December 4, 2024 17:42
@Tukks Tukks marked this pull request as ready for review December 4, 2024 17:43
@Tukks
Copy link
Author

Tukks commented Dec 4, 2024

This is a first implementation, I will keep it simple for the moment.
Later I will add codeblock, Headings, and other format who I think can be interesting in Hoarder.

here's a full implementation example : https://playground.lexical.dev/?showTreeView=false&isAutocomplete=true

Let me know if everything all right in the PR.

@Tukks Tukks force-pushed the feat/fulle-featured-markdown-editor branch 3 times, most recently from 0400ae9 to b687d6f Compare December 5, 2024 07:43
@Tukks
Copy link
Author

Tukks commented Dec 5, 2024

Added useTranslation to all string

First implementation with a wysiwyg markdown editor.
Update:
- Add Lexical markdown editor
- consistent rendering between card and preview
- removed edit modal, replaced by preview with save action
- simple markdown shortcut: underline, bold, italic etc...
@Tukks Tukks force-pushed the feat/fulle-featured-markdown-editor branch from b687d6f to 19f0427 Compare December 5, 2024 14:52
improved performance to not rerender all note card when one is updated
@Tukks Tukks force-pushed the feat/fulle-featured-markdown-editor branch from 812abbc to e3bc06d Compare December 5, 2024 20:20
@MohamedBassem
Copy link
Collaborator

Ok, I spent a lot of time today reading about lexical and understanding how it works. It's pretty cool and thanks for introducing it into the codebase. I've pushed a bunch of modifications/fixes to the PR that I hope you don't mind:

  1. Dropped alignment and underline support as those are not supported in markdown.
  2. Dropped the history undo/redo buttons for now just to simplify things a bit. Kept the history plugin though as the keyboard shortcuts still work.
  3. Added markdown shortcuts such that the markdown is rendered as you type it.
  4. Fixed a bug in the toolbar not correctly reflecting highlights and code.

Now, to merge this PR. There's a couple of things that we need to do:

  1. I don't like the flicker that happens in the bookmark cards in homepage on load. Seems like lexical only works on the client side (Feature: Server-Side Rendering (SSR) Support for Lexical Editor facebook/lexical#4960). Which means that I think we should continue using react-markdown in the bookmark cards to avoid this flicker. Unfortunate but can't find a way around it.
  2. We don't actually need to use a theme. Just adding prose dark:prose-invert prose-p:m-0to theContentEditablegets us most of the way there. The benefit of that is that it'll make the styling look similar toreact-markdown` and to what it'll look like on the mobile app.
  3. Not a blocker, but I think we should move the Save button to the toolbar.
  4. We should add an escape hatch in the editor to toggle editing raw markdown so that people don't get frustrated from fighting with the editor.

@Tukks
Copy link
Author

Tukks commented Dec 9, 2024

Hello,

Thank for your feedback and review.
I saw you removed the UpdateMarkdownPlugin, it was needed when you updated the markdown in the editor to make change appear in the grid. I will not be needed if we use react-markdown on the grid.

  • Dropped alignment and underline support as those are not supported in markdown.

You're right.

  • Dropped the history undo/redo buttons for now just to simplify things a bit. Kept the history plugin though as the keyboard shortcuts still work.

No problem, I understand.

  • Added markdown shortcuts such that the markdown is rendered as you type it.

Yes, I will add a little tooltip helper so

  1. I don't like the flicker that happens in the bookmark cards in homepage on load. Seems like lexical only works on the client side

Yes, it bother me too, I didn't know how we can find a way around it.
I will give it a try with react-markdown and prose dark:prose-invert prose-p:m-0 (I did'nt understand how this can work, but let see)

  1. Not a blocker, but I think we should move the Save button to the toolbar.

Why not, I have no opinion on that.

  1. We should add an escape hatch in the editor to toggle editing raw markdown so that people don't get frustrated from fighting with the editor.

Definitely yes, you can see a button in the lexical playground here

@Tukks
Copy link
Author

Tukks commented Dec 11, 2024

Hello,
Make some change :

  • added ListPlugin because if absent, there's a bug where you can't escape a list with enter + enter
  • added codeblock plugin
  • added prose dark:prose-invert prose-p:m-0 like you said (there's room for improvement I think, don't took the time to deep dive in) and removed theme
  • Added a switch to show raw markdown
  • Added back the react markdown for card (SSR)

Todo ?

  • Save button in toolbar ?
  • Some testing

Giuseppe Lapenta added 2 commits December 11, 2024 17:45
…escape a list with enter + enter

    - added codeblock plugin
    - added prose dark:prose-invert prose-p:m-0 like you said (there's room for improvement I think, don't took the time too deep dive in) and removed theme
    - Added a switch to show raw markdown
    - Added back the react markdown for card (SSR)
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