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

Align headers and typography in the theme #1627

Merged
merged 3 commits into from
Feb 16, 2023
Merged

Align headers and typography in the theme #1627

merged 3 commits into from
Feb 16, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Feb 2, 2023

Fixes #1612

The approach here was to teach the markdown element about the typography styles of the theme. Then define the headers sizes in the theme. That way the theme is the single source of truth for bot the Markdown and the Text components.

before:

Screenshot 2023-02-02 at 17 40 52

after:

Screenshot 2023-02-02 at 17 39 48

This also removes the margins for first and last headers. Anyone with better eye for design, feel free to suggest changes. @gerdadesign ?

@oliviertassinari oliviertassinari requested a deployment to md-headers - toolpad-db PR #1627 February 2, 2023 16:40 — with Render Abandoned
@Janpot Janpot added the feature: Components Button, input, table, etc. label Feb 2, 2023
@oliviertassinari oliviertassinari temporarily deployed to md-headers - toolpad PR #1627 February 2, 2023 16:56 — with Render Destroyed
@Janpot Janpot marked this pull request as ready for review February 3, 2023 07:43
@Janpot Janpot requested a review from a team February 3, 2023 12:11
@gerdadesign
Copy link
Member

Curious about the values used for H1, H2, etc. Seems different than the expectation to use the Material typographic scale.

@Janpot
Copy link
Member Author

Janpot commented Feb 7, 2023

Seems different than the expectation

To me, the Material typographic scale doesn't feel dense enough for the use-cases we intend Toolpad for. I tried to align a bit to what the MUI docs do. Happy to revert to the defaults. I'm not quite sure yet how we ideally should handle this.

@oliviertassinari oliviertassinari requested a deployment to md-headers - toolpad-db PR #1627 February 8, 2023 12:15 — with Render Abandoned
@oliviertassinari oliviertassinari temporarily deployed to md-headers - toolpad PR #1627 February 8, 2023 12:16 — with Render Destroyed
@prakhargupta1
Copy link
Member

Looks good to me and I agree that Material typography scale is not ideal for Toolpad use cases. Retool and appsmith also seem to have a smaller scale.

Q) Now that H1 is bold, if I want to use that size but not the bold variant, is there anyway I can do it through sx? I searched for it but couldn't find.

@Janpot
Copy link
Member Author

Janpot commented Feb 8, 2023

Q) Now that H1 is bold, if I want to use that size but not the bold variant, is there anyway I can do it through sx? I searched for it but couldn't find.

@prakhargupta1 it can be done on a case by case basis like so https://toolpad-pr-1627.onrender.com/_toolpad/app/cldvok57d0001ql9utit3cf7t/pages/ev03mab
But ideally we support this in theming and allow you to override this style once for the whole application.

Looks good to me and I agree that Material typography scale is not ideal for Toolpad use cases.

@gerdadesign If you agree and feel like playing around with a new scale, denser, for Toolpad specifically, I'd be happy to include your suggestions. If you think we should maintain the Material design scale I'm also happy to revert. We can also bring it up for discussion next meeting.

@Janpot
Copy link
Member Author

Janpot commented Feb 16, 2023

Merging this, we can always tweak later

@Janpot Janpot merged commit 102427d into master Feb 16, 2023
@Janpot Janpot deleted the md-headers branch February 16, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Components Button, input, table, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align the styling of the Text component variant with the Markdown component
5 participants