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

[docs] Use IBM Plex Sans in Tailwind CSS demos #38464

Merged
merged 8 commits into from
Aug 15, 2023

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova added docs Improvements or additions to the documentation package: base-ui Specific to @mui/base labels Aug 14, 2023
@mui-bot
Copy link

mui-bot commented Aug 14, 2023

Netlify deploy preview

https://deploy-preview-38464--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against cce086a

@mnajdova
Copy link
Member Author

mnajdova commented Aug 14, 2023

@zanivan can you check the demos to validate the font changes, please?


On another note, I noticed that when the custom font family is added in sandbox it breaks the custom tailwind config, however, it works perfectly fine in the docs (broken sandbox - https://codesandbox.io/s/ymys9l?file=/public/index.html:1549-1575 fonts are not working, but also none of the custom tailwind config). It's likely related to the whole sandbox + create react app + CDN tailwind CSS setup. I am removing it for now, but it may be interesting to consider moving the sandbox and stackblitz demos to use vite instead of create-react-app. cc @mui/docs-infra

@@ -1,60 +0,0 @@
import * as React from 'react';
Copy link
Member Author

Choose a reason for hiding this comment

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

These files were not removed when the Tailwind CSS + plain CSS demos were added.

tailwind.config = {
theme: {
extend: {
animation: {
Copy link
Member Author

Choose a reason for hiding this comment

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

These are just ordered alphabetically

@@ -19,7 +19,7 @@ const CustomButton = React.forwardRef((props, ref) => {
<Button
ref={ref}
className={clsx(
'cursor-pointer disabled:cursor-not-allowed text-sm bg-violet-500 hover:bg-violet-600 active:bg-violet-700 text-white rounded-md font-semibold px-4 py-2 border-none disabled:opacity-50',
'cursor-pointer disabled:cursor-not-allowed text-sm font-sans bg-violet-500 hover:bg-violet-600 active:bg-violet-700 text-white rounded-lg font-semibold px-4 py-2 border-none disabled:opacity-50',
Copy link
Member Author

Choose a reason for hiding this comment

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

I also fixed the wrong border radius, I just noticed the difference.

@zanivan
Copy link
Contributor

zanivan commented Aug 14, 2023

@zanivan can you check the demos to validate the font changes, please?

Only number input, tabs and form control seem not to be loading IBM plex on Tailwind demos, all the others are ok 👍

@mnajdova mnajdova requested a review from zanivan August 15, 2023 06:34
@mnajdova mnajdova marked this pull request as ready for review August 15, 2023 08:58
@mnajdova
Copy link
Member Author

Only number input, tabs and form control seem not to be loading IBM plex on Tailwind demos, all the others are ok 👍

Nice catch! Fixed

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 15, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 15, 2023
Copy link
Contributor

@zanivan zanivan left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Comment on lines +32 to 34
fontFamily: {
sans: ['IBM Plex Sans', ...defaultTheme.fontFamily.sans],
},
Copy link
Member

Choose a reason for hiding this comment

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

This makes me think that at one point we could consider having an MUI theme to Tailwind CSS theme converter like in https://reshaped.so/content/docs/getting-started/integrations/tailwind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants