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

React Quill Markdown #3246

Merged
merged 12 commits into from
Apr 1, 2023
Merged

React Quill Markdown #3246

merged 12 commits into from
Apr 1, 2023

Conversation

rbennettcw
Copy link
Contributor

@rbennettcw rbennettcw commented Mar 23, 2023

Adds markdown support to the ReactQuillEditor component and updates parent components to support the new functionality via serialization/deserialization of text.

Link to Issue

Closes: #2933

Description of Changes

  • Adds M/R (Markdown/Richtext) button to editor to switch between modes
  • Adds preview button which opens preview modal on click
  • Updates parent components to use serialize/deserialize logic which helps with parsing markdown vs richtext

Test Plan

  • Create a new comment (richtext by default)– confirm that it formats and submits correctly
  • Create a new comment and switch to markdown mode– confirm that it formats and submits correctly
  • Create a new comment, switch between the modes, and click the preview button– confirm the preview reflects the current mode
  • Click Edit on a richtext comment– confirm that the editor opens in richtext mode
  • Click Edit on a markdown comment– confirm that the editor opens in markdown mode
  • Other affected areas include: Create Topic, Edit Topic, Create Thread, Edit Thread

Deployment Plan

N/A

Other Considerations

  • Currently, the editor toolbar buttons don't work correctly in markdown mode, but markdown support otherwise works correctly

@rbennettcw rbennettcw marked this pull request as draft March 23, 2023 21:25
@rbennettcw
Copy link
Contributor Author

Done:

  • Added markdown toggle button and preview button
  • Converted preview modal to modern React and added visibility logic

Next steps:

  • Get preview modal to render markdown/richtext correctly
  • Ensure that the toggle behavior works as expected

@rbennettcw
Copy link
Contributor Author

The editor now supports markdown and richtext– the last important thing to do is handle serializing for saving to the DB.

Currently, the editor state is represented only as a StaticDelta object. When it comes to persistence, markdown is stored as plain text while richtext is stored as a JSON string.

I'm working on handling this without increasing complexity outside the react quill component.

@rbennettcw rbennettcw marked this pull request as ready for review March 28, 2023 15:11
@rbennettcw
Copy link
Contributor Author

rbennettcw commented Mar 28, 2023

Markdown support generally works now and I think this is ready to merge.

One thing that's unresolved: toolbar buttons don't work properly in markdown mode. I'm working on it, but I think maybe it can be pulled out into another ticket and this PR can be merged as-is so we can get the code out into the wild and tested.

Also note: there's a small flash when switching between markdown and richtext mode, due to a bug in the React Quill library that forces us to use a workaround to rerender the component. There's a potention fix for it, but it's not part of the main branch of the library: zenoamaro/react-quill#838

@rbennettcw
Copy link
Contributor Author

Pausing further work on this due to: #2900

@jnaviask jnaviask linked an issue Mar 30, 2023 that may be closed by this pull request
@masvelio
Copy link
Contributor

Pausing further work on this due to: #2900

That's a bummer. @rbennettcw Let's close the PR and hopefully get back to it in post-react

@rbennettcw
Copy link
Contributor Author

@jnaviask Agreed that this should probably be closed.

Although markdown is technically implemented in an MVP state, this PR will require a bit more work to reach full parity with production, and some testing across multiple impacted features.

If velocity and stability is the priority, then it's best not to merge this, and probably to close it.

@CowMuon CowMuon merged commit 1fea352 into react Apr 1, 2023
@CowMuon CowMuon deleted the ryan.react-quill-markdown1 branch April 1, 2023 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quill: add support for markdown
4 participants