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

v1 of the editor #9143

Merged
merged 38 commits into from
Sep 9, 2024
Merged

v1 of the editor #9143

merged 38 commits into from
Sep 9, 2024

Conversation

burtonator
Copy link
Collaborator

@burtonator burtonator commented Sep 6, 2024

Link to Issue

Closes: #9144

Description of Changes

All the v1 issues documented here:

#8937

But here are the highlights.

  • better drag/upload indicators
  • mobile/desktop layouts now work
  • bugs from v0 fixed
  • /editor has a desktop and mobile version you can use.

"How We Fixed It"

Test Plan

  • There's a test plan in the README.md

You can just load /editor in the UI and play with it.

Deployment Plan

Other Considerations

@burtonator burtonator marked this pull request as ready for review September 6, 2024 19:07
@burtonator burtonator changed the title draft: Burton/mdxeditor integration v1 v1 of the editor Sep 6, 2024
Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

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

  1. I have the same image. When I drop it, I am getting an error. When I use upload image icon, it works.
Kapture.2024-09-09.at.13.46.59.mp4
  1. Loader indicator takes up the whole page but I think it should take either the body or maximum the editor space. Also I think it should be semi-transparent as now it is all gray.

@mzparacha
Copy link
Contributor

  1. Image modal isn't responsive on small sizes (supported min screen size is 320px per this) - same with URL modal
image
  1. Uploaded image management icons are a bit misaligned
image
  1. Not sure if this is expected behavior but I think we shouldn't be allowing images to be inline
Screen.Recording.2024-09-09.at.6.35.18.PM.mov
  1. If the uploaded image is selected and dragged over, the editor enters drag/drop mode
Screen.Recording.2024-09-09.at.6.35.18.PM.mov
  1. Any URL (even non-image ones) works when uploading an image through a link
Screen.Recording.2024-09-09.at.6.38.59.PM.mov

Copy link
Contributor

@mzparacha mzparacha left a comment

Choose a reason for hiding this comment

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

Found some minor issues above, generally works. Thanks!

@burtonator
Copy link
Collaborator Author

@masvelio on your issue with inline images, I think we have to support that.

I agree that it can be kind of annoying, but:

  1. I'm not sure how we would block that. I think I'd have to really hack the MDXEditor internals to do that.

  2. It's actually valid HTML and there are reasons to do this like if you want a little image inline in your text.

I agree it's rare though but the downside is you just undo the operation.

@burtonator
Copy link
Collaborator Author

burtonator commented Sep 9, 2024

@masvelio Your comment about "If the uploaded image is selected and dragged over, the editor enters drag/drop mode"

... I think this is resize mode. Where you can resize the image.

This is expected.

@burtonator
Copy link
Collaborator Author

@masvelio "Loader indicator takes up the whole page but I think it should take either the body or maximum the editor space. Also I think it should be semi-transparent as now it is all gray."

You have a stale build or something. That's from v0... I've fixed that. You could probably pull my new branch and test again. Not sure how that happened.

@burtonator
Copy link
Collaborator Author

@masvelio

Uploaded image management icons are a bit misaligned

They're aligned to the right but your image has some white background area, correct?

image

You can see them better here.

PS. Sorry about the Putin image!

@burtonator burtonator merged commit 897e5bd into master Sep 9, 2024
10 checks passed
@masvelio
Copy link
Contributor

@burtonator you should mention @mzparacha in post of this cases, not me. Anyways, let me answer some of them.

They're aligned to the right but your image has some white background area, correct?

You can see them better here.

PS. Sorry about the Putin image!

They are not aligned horizontally. Trash icon is above the cog icon, they should be on the same level.

  1. Any URL (even non-image ones) works when uploading an image through a link

@burtonator this is not fixed ☝️

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 v1 of the editor
3 participants