Polish markdown component

mui / toolpad

Toolpad: Full stack components and low-code builder for dashboards and internal apps.

https://mui.com/toolpad/

MIT License

1.27k stars 282 forks source link

Polish markdown component #1470

Closed oliviertassinari closed 1 year ago

oliviertassinari commented 1 year ago

Duplicates

[X] I have searched the existing issues

Latest version

[X] I have tested the latest version

Steps to reproduce 🕹

Steps:

Open https://master--toolpad.mui.com/deploy/cl6rqzry10009arlv9sto6qja/pages/e943n9y

Current behavior 😯

There is a margin around the markdown.
The <a> isn't rendered with the <Link> of Material UI.

Expected behavior 🤔

The opposite.

Context 🔦

I'm updating some of the Toolpad app we use for the support to use markdown. Related to #1298.

Your environment 🌎

No response

oliviertassinari commented 1 year ago

I wonder what we should do with the headers. At minimum, they should use the theme typography but not sure about what's the right variants for them.

oliviertassinari commented 1 year ago

I noticed something else while using this new markdown feature. In https://master--toolpad.mui.com/_toolpad/app/cl6rqzry10009arlv9sto6qja/pages/ep03m95

when I click on the link, it breaks the editor and doesn't open the link. I clicked on the link to see if I needed to update the link or not.

We could add target="_blank". It wasn't smooth.

Janpot commented 1 year ago

There is a margin around the markdown.

I think it's the spacing that's between all elements on the page. Perhaps it makes sense to add the title inside of the markdown as well?

bharatkashyap commented 1 year ago

There is a margin around the markdown.

I think it's the spacing that's between all elements on the page.

There definitely was some additional margin that react-markdown was adding, markdown-to-jsx fixed that

oliviertassinari commented 1 year ago

On https://master--toolpad.mui.com/_toolpad/app/cl6rqzry10009arlv9sto6qja/pages/ip23ggo, it feels cleaner, great 👍

Things that we could still improve:

[x] The margin between paragraph feels too small. It's currently at 6px. For example, GitHub uses 16px, MUI docs too. Yes Retool uses 8px but it's because their text font-size is 12px, not 16px like us.
[x] Fix the leading margin-bottom: https://github.com/mui/mui-toolpad/pull/1477/files/66acedbede09680e45e593d84504d4c2391e53e5#r1064221934

I'm reopening, we can use this as an umbrella issue. There will likely be a lot of opportunities for polish if the becomes used. For now, it's not really important to improve the markdown component.

Janpot commented 1 year ago

Things that we could still improve:

Adjusted those in https://github.com/mui/mui-toolpad/pull/1690

bharatkashyap commented 1 year ago

@prakhargupta1 Should we close this since the to-dos listed in the comment above have been closed? We can potentially create another issue about separating the Text and the Markdown components back into two different ones (This is the one: https://github.com/mui/mui-toolpad/issues/1702)