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

v3 of the editor #9288

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

v3 of the editor #9288

wants to merge 53 commits into from

Conversation

burtonator
Copy link
Collaborator

@burtonator burtonator commented Sep 19, 2024

This also duplicates NewThreadForm into NewThreadFormLegacy and NewThreadFormModern so you don't really need to fully audit those two as NewThreadFormModern is the main one and most of the code didn't change.

Also, everything here should be safely behind a feature toggle so nothing is being turned on yet.

I have 1-2 more revisions to go before we can turn it on.

Link to Issue

Closes: #9290

Also fixes issues:

Description of Changes

  • See the v3 issues link above.

"How We Fixed It"

Test Plan

The test plan is in the README.md but here is the list.

I would not do this yet though. We can do a full QA and test of everything right before we're ready to turn everything on next week.

Desktop

  • success: copy a .md file to the clipboard, try to paste it into the editor. It
    should insert the content at the editor's cursor

    • this works via a File object (not text) so it's important to test this path.
  • success: drag a .md file on top of the editor. The drag indicator should show
    up and cover the editor while you're dragging. Then the file should be inserted
    at the cursor.

  • success: use the 'Import markdown' button to upload a file.

  • success: right click and copy an image in the browser, this should upload it
    to the editor and insert it at the current point (I use msnbc.com for this as
    their images are copyable and not CSS background images)

  • success: take a screenshot, try to paste it into the editor. The upload
    indicator should show up.

  • success: use the image button at the top to manually upload an image. The
    upload indicator should show up while this is happening.

  • success: drop an image file. Should upload it for us and not handle it as
    markdown.

  • failure: copy multiple .md files ot the clipboard, try to paste into the editor.
    It should fail because we can't handle multiple .md files

Mobile

It's probably best to test this on a REAL mobile browser (not on a desktop).

  • The toolbar should be present at the bottom of the UI.

  • They keyboard should stay on top of the keyboard.

Viewer

  • make sure images, tables, and code don't have UI control elements

Deployment Plan

Other Considerations

… closer to having it implemented in all places.
- another component ported to MarkdownViewer
- can compare layout to quill now.
# Conflicts:
#	packages/commonwealth/client/scripts/views/components/NewThreadForm/NewThreadForm.tsx
#	packages/commonwealth/package.json
#	pnpm-lock.yaml
@burtonator burtonator changed the title draft: Burton/editor v3 v3 of the editor Sep 19, 2024
@burtonator burtonator marked this pull request as ready for review September 19, 2024 19:12
@masvelio
Copy link
Contributor

@burtonator Mobile issues tested on iphone:

  1. Bottom toolbar is not visible when i want to start typing and the keyboard shows up
  2. I am not able to set the cursor with my finger and select the range of text
153378AA-24C2-4292-B485-0D97E3D72673.MP4

# Conflicts:
#	packages/commonwealth/client/scripts/views/components/NewThreadForm/NewThreadForm.tsx
@burtonator
Copy link
Collaborator Author

@masvelio The browser you used is mobile Safari right ?

I don't get those issues on Chrome but I do get some different issues.

I'll pull out my tablet/iphone and test there too but I'm going to do it in v4.

Could you approve this PR though so we can push forward?

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.

Implement all v3 issues for the editor.
2 participants