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

Polish markdown component #1470

Closed
2 tasks done
oliviertassinari opened this issue Dec 19, 2022 · 7 comments · Fixed by #1477
Closed
2 tasks done

Polish markdown component #1470

oliviertassinari opened this issue Dec 19, 2022 · 7 comments · Fixed by #1477
Assignees
Labels
design: ui Design enhancement This is not a bug, nor a new feature umbrella For grouping multiple issues to provide a holistic view

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 19, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Steps:

  1. 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.

Screenshot 2022-12-19 at 12 16 23

Expected behavior 🤔

The opposite.

Screenshot 2022-12-19 at 12 16 52

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 oliviertassinari added status: waiting for maintainer These issues haven't been looked at yet by a maintainer bug 🐛 Something doesn't work design: ui Design and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 19, 2022
@bharatkashyap bharatkashyap self-assigned this Dec 19, 2022
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 19, 2022

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
Copy link
Member Author

oliviertassinari commented Dec 19, 2022

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

Screenshot 2022-12-19 at 16 54 46

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.

Screenshot 2022-12-19 at 16 52 34

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

@Janpot
Copy link
Member

Janpot commented Dec 20, 2022

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
Copy link
Member

bharatkashyap commented Dec 23, 2022

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
Copy link
Member Author

oliviertassinari commented Feb 21, 2023

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

Things that we could still improve:

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.

@oliviertassinari oliviertassinari added umbrella For grouping multiple issues to provide a holistic view enhancement This is not a bug, nor a new feature and removed bug 🐛 Something doesn't work labels Feb 21, 2023
@Janpot
Copy link
Member

Janpot commented Feb 21, 2023

Things that we could still improve:

Adjusted those in #1690

@bharatkashyap
Copy link
Member

bharatkashyap commented Apr 19, 2023

@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: #1702)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design: ui Design enhancement This is not a bug, nor a new feature umbrella For grouping multiple issues to provide a holistic view
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants