market icon indicating copy to clipboard operation
market copied to clipboard

add MDEditor in Publish / Edit

Open EnzoVezzaro opened this issue 2 years ago • 13 comments

  • Fixes #1496 .

Changes proposed in this PR:

  • Added MDEditor in Description field
Screenshot 2022-08-22 at 07 19 47

WIP:

  • [X] finish logic & styling
  • [X] make sure "all markdown previews we have in our app renders exactly like we render the final asset details."
  • [X] add tabs for preview
  • [X] light / dark mode
  • [x] bug: in long texts - after 500 words or so, it splits into a weird transparent editor (if you select you see the text) - WIP

For testing:

  • Markdown Editor: -- Publish Form -- Edit Form
  • Mardown Preview -- Publish (preview step) -- Edit Form (preview tab) -- Asset Details -- Asset Teaser

EnzoVezzaro avatar Jul 13 '22 11:07 EnzoVezzaro

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
market ✅ Ready (Inspect) Visit Preview Aug 31, 2022 at 0:59AM (UTC)

vercel[bot] avatar Jul 13 '22 11:07 vercel[bot]

We do not want editor and preview side-by-side, there is not enough space to not make editing feel claustrophobic once you have an actual paragraph written. If we want a preview there, it should follow the GitHub UI pattern with Write/Preview tabs. And if we use description preview alone, then we also need to make sure the way we render markdown for our asset preview is the same as what this markdown editor renders. In the end, all markdown previews we have in our app should render exactly like we render the final asset details.

kremalicious avatar Jul 13 '22 12:07 kremalicious

it is like github, we just need to remove middle button image

mihaisc avatar Jul 13 '22 12:07 mihaisc

As we basically just want the formatting buttons, maybe https://github.com/github/markdown-toolbar-element is a better choice for solving this. Then we don’t need to deal with multiple ways of rendering markdown or the potential overhead currently choosen dependency might bring

kremalicious avatar Jul 13 '22 12:07 kremalicious

As we basically just want the formatting buttons, maybe https://github.com/github/markdown-toolbar-element is a better choice for solving this. Then we don’t need to deal with multiple ways of rendering markdown or the potential overhead currently choosen dependency might bring

  • changes on #1599

EnzoVezzaro avatar Jul 14 '22 11:07 EnzoVezzaro

If we want to make this even more friendly for non-developers we could user a wysiwyg editor that can export the content as markdown. Something like this looks like it could do the trick

jamiehewitt15 avatar Aug 16 '22 14:08 jamiehewitt15

If we want to make this even more friendly for non-developers we could user a wysiwyg editor that can export the content as markdown. Something like this looks like it could do the trick

Thanks @jamiehewitt15. I wouldn't introduce another editor as for now, even though I like it 😅 Both editors (MDEditor / Markdown-toolbar) actually do the trick, but they don't solve all the UX problems. Here, for example, seems to be appreciated the split screen functionalities, so makes more sense for us to keep working with MDEditor. What we discussed earlier in standup should be enough for our use case, but maybe @kremalicious has a better opinion on this.

EnzoVezzaro avatar Aug 16 '22 15:08 EnzoVezzaro

Being struggling to style this component, any change requires some tricky CSS way to do it. Also, it should handle dark-mode, but doesn't seem to work at all. I've spent too much time trying to make it work, but I haven't being able to. This affects not dark mode, but also ignores light theme's color scheme.

Screenshot 2022-08-19 at 07 41 54 Screenshot 2022-08-19 at 07 46 10

Considering we also have the problem with next-remove-imports, I would be more inclined to change to @jamiehewitt15 's suggestion or similar.

UPDATE: online (deployment) dark-mode seems to be working as for default (not custom colors, but default gets picked up), can't figure out why on localhost is not actually working.

Screenshot 2022-08-19 at 08 23 17

EnzoVezzaro avatar Aug 19 '22 12:08 EnzoVezzaro

Considering we also have the problem with next-remove-imports, I would be more inclined to change to @jamiehewitt15 's suggestion or similar.

Yeah, might be worth giving the other package a go to see if it's easier to implement and work with

jamiehewitt15 avatar Aug 19 '22 12:08 jamiehewitt15

Agree with Enzo, no new editor trying to solve all use cases, and WYSIWYG editors introduce a whole set of new problems. To me, this is what we want to solve here, and nothing more:

  1. previewing description is the most important preview, yet the preview step is not easy to reach
  2. some people are not used to Markdown but still want to do small formatting

For solving 1, it absolutely requires that we render the markdown the same way we render the big preview/final description. We have methods and components for that. This can‘t be solved with another rendering of some plugin. In theory we do not even need a dedicated markdown preview plugin here, we have that already with our own components.

Then, UI-wise, not sure we want to go into creating own custom UI by just copying the layout of GitHub comments authoring. As long as we manage to separate write & preview and not have it side-by-side, it should be good

kremalicious avatar Aug 19 '22 12:08 kremalicious

haven't find a way to fix this:

  • bug: in long texts - after 500 words or so, it splits into a weird transparent editor (if you select the text you'll see it)
Screenshot 2022-08-22 at 07 22 41

It should work as expected by default (in the docs works), but our project seems to have something that the component doesn't like. It might just be some CSS property messing up the component's style.

UPDATE: solved

EnzoVezzaro avatar Aug 22 '22 13:08 EnzoVezzaro

quick review so it doesn't get merged with all those problems. In general, again, the task here is not to figure out which plugin to add. The task is to solve 2 main problems for the description authoring, where we might use a plugin. Just adding a plugin leads to new problems without solving the actual mentioned problems:

  • preview is the most important thing we want to solve here, yet the button for it is hidden and not clear at all. And impossible to hit on mobile, which holds true for all buttons in that toolbar, unusable on mobile.
  • editor and preview use different typography
  • editor and preview use different colors, like suddenly there's a lot of blue text which we never use in the UI.
  • editor preview uses its own markdown rendering, independent from how we render the final description, which is exactly what we want to prevent. The preview here, the big publish preview, and the final page all need to have the same rendering, otherwise it's not a preview. Markdown is not Markdown, there are many fine differences in different libraries, we try to always use GitHub flavored Markdown which all our existing Markdown components do. Stylistically the current preview renders our markdown completely different than our actual rendering, completely different styles like quote style and so on. Again, theoretically we do not need a plugin for the preview, we can do it on our own as this PR already creates a custom component with full control over markup.
  • button starts titles at h1. Look at the final page and see which headings are already taken by other elements like the main title, then adapt editor buttons to only include titles starting from that hierarchy. Most likely h2 or h3. We can't have multiple h1 on the final asset details page. Would opt for having only 1 level of title hierarchy as GitHub does (and they make it an h3) as those buttons take away too much space but that's debatable

Examples: this is what the current editor preview shows me:

Screen Shot 2022-08-23 at 11 32 19

This is what we show in publish preview and final thing, completely different so we would only confuse users. Note how GFM deals differently with line breaks at the end:

Screen Shot 2022-08-23 at 11 31 20

From a UX perspective this is just perfect so if we can't come up with something better solving our 2 main problems, please just copy this:

Screen Shot 2022-08-23 at 11 22 40

kremalicious avatar Aug 23 '22 10:08 kremalicious

Code Climate has analyzed commit 04920cca and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 6.5% (0.0% change).

View more on Code Climate.

codeclimate[bot] avatar Aug 31 '22 13:08 codeclimate[bot]

we will stop working on this. Too much effort. Wil not delete the branch, maybe we will continue at a later time.

mihaisc avatar Sep 29 '22 14:09 mihaisc