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] Add customization demos #27411

Merged
merged 30 commits into from
Jul 28, 2021
Merged

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Jul 23, 2021

Summary
Add Theming section to these pages. (to be used in rebranding).

Screen Shot 2564-07-23 at 16 48 00

Screen Shot 2564-07-23 at 16 48 12

Screen Shot 2564-07-23 at 16 48 21

Screen Shot 2564-07-23 at 16 49 07

Screen Shot 2564-07-23 at 16 51 13

Screen Shot 2564-07-23 at 16 52 01

Screen Shot 2564-07-23 at 16 52 42

Screen Shot 2564-07-23 at 16 53 41

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 23, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 7a3ec82

@siriwatknp siriwatknp changed the title [docs] Add customization card demos [docs] Add customization demos Jul 23, 2021
@michaldudak
Copy link
Member

🚀 Looks great! I love the subtle gradient on the card!

@mnajdova
Copy link
Member

Looks great, some ideas for improvements:

When I hover on the icon for opening the years, the hover ring is cut:

image

Is there enough contrast here, my eyes have a bit hard time reading it (same for the Card)

image

They look great! I am excited :D

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

  1. We are asking for PlusJakartaSans, but we don't load it.
  2. What's the long term plan around this theming section? It's it only here temporarily before moving it to the marking pages? Is the plan to add a section for theming like we have one for customization on all the pages?

docs/src/pages/components/cards/PlayerCard.tsx Outdated Show resolved Hide resolved
docs/src/pages/components/date-picker/date-picker.md Outdated Show resolved Hide resolved
docs/src/pages/components/cards/NotificationCard.js Outdated Show resolved Hide resolved
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@siriwatknp
Copy link
Member Author

  • We are asking for PlusJakartaSans, but we don't load it.

will remove it in this PR, and add it in rebranding.

@siriwatknp
Copy link
Member Author

What's the long term plan around this theming section?

I think it is nice to have a theming section in a component page apart from customization with sx or styled to give a glimpse of how theming works and then lead to full theming documentation. We can also measure in analytics in the future whether the assumption is correct or not.

@@ -105,6 +105,12 @@ This might be less confusing to users compared to a change in direction.

{{"demo": "pages/components/slider/VerticalAccessibleSlider.js"}}

## Theming
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the section be right after ## Customized sliders?

Also, if we move with ## Theming for the other pages, I would propose we simplify the customization header (## Customized sliders in this page) to ## Customization on all the pages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Member Author

Choose a reason for hiding this comment

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

@oliviertassinari is it better if we use the word "styling" rather than "customization"? I feel "customization" is broader than just css. @danilo-leal any thought?

Copy link
Member

Choose a reason for hiding this comment

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

We could consider too:

## Customization

### Styling

### Theming

but maybe it's too deep and a flatten structure would help.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I prefer flatten structure.

## Styling

## Theming

I think I will keep Customization and ask the team about changing to Styling so the change does not overload in this PR. @oliviertassinari should I merge the PR?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Do you want Danilo to have a look at the new demos?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that Theming should live in isolation from Customization. I believe they are about the same thing. You're customizing the components by applying a given theme in them. But, although this discussion is very important, I don't feel like this is the PR/place to discuss it already.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danilo-leal then I will remove them from the theming section and merge until we have a conclusion.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Jul 27, 2021
Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

This looks very good! I've made just small tweaks to them, especially regarding the cards in dark mode and the tabs contrast. You can check on the Figma file for quick scanning.

@siriwatknp siriwatknp force-pushed the docs/card-customization branch 3 times, most recently from 46b0452 to 29f1f7d Compare July 28, 2021 06:34
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants