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

Investigating migration away from Quill #7089

Open
burtonator opened this issue Mar 13, 2024 · 12 comments
Open

Investigating migration away from Quill #7089

burtonator opened this issue Mar 13, 2024 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@burtonator
Copy link
Collaborator

burtonator commented Mar 13, 2024

I'm currently investigating helping out @lzach83 migrate us away from Quill and trying to help out where I can.

Looks like there's a ton of work here so I really appreciate all that you guys have done to try to migrate us away!

I'm currently using your current research here:

Kooking through everything, I think we should probably pick the best markdown editor we can find. I think the main requirements should probably be the developer experience and the ability to work well on mobile. I think we should probably be less worried about advanced features though and just try to get basic markdown working.

Some of these advanced features like mermaid.js charting and katex support are super cool but they're not being actively requested by customers. However, devx and mobile support is biting us right now.

We're already experiencing pain from our existing editor and this will give us a path forward. I think long term we might have to implement our own editor though and we're not going to be able to implement concurrent editing anytime soon but I don't think we have to necessarily.

I think anything more would be a pretty herculean effort. I think this is a good medium term path though. It's worth the investment, it's something we can do in the short term, we will see a big win because of it, etc.

@burtonator
Copy link
Collaborator Author

My current path is to dive into the code a bit and get a working version to test the mobile support and make sure the devx is solid.

@burtonator
Copy link
Collaborator Author

I think the editor should allow generally raw markdown generation but on the backend we're going to need a markdown sanitizer to prevent any potential XSS attacks. We probably need to do this in the HTML layer so that any HTML is sanitized before going into the UI

@burtonator burtonator added the enhancement New feature or request label Mar 13, 2024
@burtonator
Copy link
Collaborator Author

I think I need some more feedback from the team here. @ForestMars @dillchen @zakhap @jessmart1213

(I tried to tag everyone here but could you please add anyone else you think should provide feedback?)

This is our current editing control:

Image

It supports inline (raw) markdown editing and then preview.

This is backed by Quill which we're not happy with.

I've been investigating migration to one of the markdown editors listed above.

The challenge here is that there are two main editing strategies.

  • raw markdown with preview of html in a tab
  • inline editing (WYSIWYG)

Here's what inline editing looks like:

Image

This is a PoC/spike I did with MDEditor (https://mdxeditor.dev/).

I did an initial comparison here and it seems to support roughly what we need.

  • I can anchor the toolbar to the bottom so that it's above the keyboard
  • The developer experience seems straight forward
  • supports light/dark mode (this is just in dark mode here to verify it works)

I think there are two ways forward here:

  1. We could abandon quill and implement a raw markdown editor ourselves. Almost exactly what we currently need just without Quill. I think that could work.
  2. We go with MDEditor [1] and then have live preview but I think need to make a decision here.

Footnotes:

  1. I'm leaning away from react-markdown-editor now because it seems to have performance issues but still investigating.

@dillchen
Copy link
Contributor

This LGTM. I think the decision criteria I'm good with. And demo looks good

I believe there is emerging consensus from team on mdx. Some caveats.

I am still curious to hear my answers on the previous GitHub thread. How hard the dev ex is for adding the advanced features.

Not a blocker, just wanna know we aren't gonna shoot ourselves in the foot trying to extend to more "modern" features

@jnaviask
Copy link
Collaborator

@burtonator my personal preference here would be to not support WSIWYG x markdown, because in my mind the combination doesn't make sense. Markdown is specifically designed to syntactically reflect the outputs when viewed as raw text, so in theory a preview window should be sufficient. The outstanding question, though, is whether are users should deal with raw markdown or whether they would prefer a richtext experience backed by markdown, which is a question for product / design.

@lzach83
Copy link
Contributor

lzach83 commented Mar 14, 2024

@burtonator, I share Jake's perspective. Implementing WYSIWYG and Markdown would introduce significant overhead without necessarily delivering equivalent results. Regarding the rich text experience, our design team has been developing a Figma prototype(mostly mobile for now) showcasing features resembling rich text editing. Although Zak or Jessica would provide definitive answers, indications suggest that we're moving towards a "rich text experience powered by markdown" approach.

@jnaviask
Copy link
Collaborator

@burtonator and cc @jessmart1213 @zakhap we should put engineering-side research on pause for the time being until we receive feedback from product/design wrt what option (WYSIWYG vs Raw Markdown) we want to target for our eventual release

@burtonator
Copy link
Collaborator Author

@jnaviask @zakhap @jessmart1213 I think this is the main issue. Do we go with WYSIWYG or raw markdown editing.

I don't really have any strong feelings here just that we should try to get something out soon so that we offload Zack by fixing these bugs by moving to a new platform.

I almost have the PoC of the editor done for the WYSIWYG mode.

I think the main argument for WYSIWYG from my perspective is probably tables. Tables in raw markdown are basically impossible to edit.

There are other features here involving how easy it work with the editor if we need features like source code editing, embeds, etc.

@burtonator
Copy link
Collaborator Author

Quick update.

MDEditor works fine in terms of features. I also have the toolbar so that it it will float above the keyboard

The main problem is that it seems that dark mode doesn't work with the dialogs for creating images.

image

Also, here's what the bottom looks like:

CleanShot 2024-03-14 at 14 43 42@2x

The main issue is WYSIWYG vs raw markdown.

I think that's the main issue.

I can fix this issue though but I don't want to spend too much time here until we go with the issue with the raw markdown vs wysiwyg

@burtonator
Copy link
Collaborator Author

burtonator commented Mar 20, 2024

@lzach83 @dillchen @ForestMars @jnaviask

I have a PoC that you guys can start to test.

Desktop version: https://commonwealth-editor-git-main-burtonator.vercel.app/
Mobile version: https://commonwealth-editor-git-main-burtonator.vercel.app/?dev=mobile

The mobile version places the toolbar at the bottom of the browser.

The main things here are:

  • All of the basic editor functionality you would expect works

    • tables
    • bold, italics, etc
    • bulleted and list items
    • creating links
    • images
    • horizontal breaks
  • Pasted images work but in production I have to switch them over to use an async
    handler that will upload the images to cloud storage.

  • Copying / pasting text with formatting works

It can support advanced source code editing, but I didn't implement that as
this was a "nice to have".

One issue is that the mobile toolbar doesn't properly scroll on mobile.

I have a fix for it, but I wanted to first get approval that this is the editor
that we should use before going down a rabbit hole.

The two fixes include:

  1. creating a fork of MDXEditor with an option to place the toolbar at the bottom
    of the editor which would elegantly solve the positioning relative to the
    keyboard on mobile devices. I think this is about 2-3 hours of work.

  2. using absolute positioning to position it at the bottom of the viewport then
    blocking browser scrolling. This is a bit of a hack so it's plan be. I'd prefer
    the first option

@burtonator
Copy link
Collaborator Author

I wanted to update our notes on the issues with pasting markdown content and whether we can detect it.

We can paste files and text/markdown is sent properly so we can import those directly.

This means drag and drop, copying the file in 'Finder' or the local file manager, or manually uploading the file via file upload.

What does not work is using the keyboard/mouse to copy text.

What happens is that no 'file' object is sent and you're sent text/plain, text/html and RTF versions of what you copied. Those have to be converted to markdown.

I think the conversion might generally work but I'm more worried about images and tables.

Also, there's an issue with "proprietary" markdown extensions like GFM (github flavored markdown) so we might have to support some of these. I'd have to spend about 1/2 to 1 day investigating this. I think the big one is probably going to be source code editing / highlighting.

@zakhap
Copy link
Collaborator

zakhap commented Apr 4, 2024

I am not particularly worried about GFM being a paint-point for users, but more so HackMD edited files as we know of explicit usage from top contributing members of communities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants