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

[joy-ui][design] Stray design tweaks to components #38476

Merged
merged 25 commits into from
Aug 31, 2023

Conversation

zanivan
Copy link
Contributor

@zanivan zanivan commented Aug 14, 2023

Added some tweaks to the components, to make them consistent and work better together out-of-the box.

Changes on default Card

  • Changed Card boxShadow to md from sm
  • Changed default Card variant to outlined
Current New
Screenshot 2023-08-14 at 14 39 53 Screenshot 2023-08-14 at 14 37 20

Changes on default Input

  • Added boxShadow to input
Current New
Screenshot 2023-08-14 at 14 41 55 Screenshot 2023-08-14 at 14 42 29

Changes on default Alert

  • Removed boxShadow from Alert
  • Changed the default Alert variant to soft
Current New
Screenshot 2023-08-14 at 14 44 04 Screenshot 2023-08-14 at 14 43 33

@zanivan zanivan added package: joy-ui Specific to @mui/joy design: joy This is about Joy Design, please involve a visual or UX designer in the process component: card This is the name of the generic UI component, not the React module! component: alert This is the name of the generic UI component, not the React module! component: input labels Aug 14, 2023
@zanivan zanivan added this to the Joy UI: Stable release milestone Aug 14, 2023
@mui-bot
Copy link

mui-bot commented Aug 14, 2023

Netlify deploy preview

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 970948d

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.

Great changes all-around ⎯ I love the subtle box shadow on the inputs, it really elevates the design a lot! 🤙 Make sure to check the failing PR tests there, though.

zanivan and others added 5 commits August 15, 2023 10:59
Co-authored-by: Siriwat K <[email protected]>
Signed-off-by: Victor Zanivan Monteiro <[email protected]>
Co-authored-by: Siriwat K <[email protected]>
Signed-off-by: Victor Zanivan Monteiro <[email protected]>
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 17, 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 17, 2023
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

One last question, since the shadow is added to Input, should it be the same for Textarea, Select, Autocomplete?

@siriwatknp
Copy link
Member

siriwatknp commented Aug 21, 2023

@zanivan I found an edge case. There are common cases where <Input variant="plain" /> or <Select variant="plain" /> are useful but not with the shadow. Now I have to do:

<Input variant="plain" sx={{ boxShadow: 'none' }} />
<Select variant="plain" sx={{ boxShadow: 'none' }} />

Should the shadow applied only to the outlined variant?

image

@zanivan
Copy link
Contributor Author

zanivan commented Aug 21, 2023

Should the shadow applied only to the outlined variant?

I think it makes sense—in the same way, I believe we should remove the shadows from outlined, soft, and solid cards. What do you think?

@siriwatknp
Copy link
Member

siriwatknp commented Aug 22, 2023

Should the shadow applied only to the outlined variant?

I think it makes sense—in the same way, I believe we should remove the shadows from outlined, soft, and solid cards. What do you think?

I think that you mean plain, soft and solid?

@zanivan
Copy link
Contributor Author

zanivan commented Aug 22, 2023

I think that you mean plain, soft and solid?

Actually no. Sorry, I expressed myself poorly, but I thought that for the cards, we should only keep the shadows on the plain variant. However, after making some tests, I think we could remove the shadows from all the cards by default. @danilo-leal I'd love to hear your take on this. I've seen many examples in libraries where, by default, the cards have shadows—but, when it comes to real products, it's commonly the opposite.

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 is looking great, overall! Agree with the reflection above ☝️ and it's been real in my experience as well with real apps. It may be generally safer to go without it by default as that can be too opinionated, devs and designers will customize that quickly if they need it!

@danilo-leal
Copy link
Contributor

@zanivan Oh, by the way, one thing I just realized while browsing the Joy UI docs that we could do here as well: I think the Chip default size could also be changed to sm. It looks too much like a pill button to me in the medium size, and Chips are usually used smaller.

@zanivan
Copy link
Contributor Author

zanivan commented Aug 23, 2023

Oh, by the way, one thing I just realized while browsing the Joy UI docs that we could do here as well: I think the Chip default size could also be changed to sm. It looks too much like a pill button to me in the medium size, and Chips are usually used smaller.

Totally! And I think even the sm could use some polishing. While I was revamping the templates, I realized that sometimes I ended up using the aspectRatio instead of the chip because it was just too big to be inside tabs and lists. I think we could polish it out on the #38527

@siriwatknp
Copy link
Member

@zanivan Oh, by the way, one thing I just realized while browsing the Joy UI docs that we could do here as well: I think the Chip default size could also be changed to sm. It looks too much like a pill button to me in the medium size, and Chips are usually used smaller.

We have discussed this and the fix to this is to change the values of md size instead of changing the default size to sm. Let's do it in #38527

Co-authored-by: Siriwat K <[email protected]>
Signed-off-by: Victor Zanivan Monteiro <[email protected]>
@zanivan zanivan requested a review from siriwatknp August 23, 2023 18:35
@zanivan zanivan merged commit 8d4728d into mui:master Aug 31, 2023
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
Signed-off-by: Victor Zanivan Monteiro <[email protected]>
Co-authored-by: Siriwat K <[email protected]>
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
Signed-off-by: Victor Zanivan Monteiro <[email protected]>
Co-authored-by: Siriwat K <[email protected]>
@oliviertassinari oliviertassinari added the component: text field This is the name of the generic UI component, not the React module! label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: alert This is the name of the generic UI component, not the React module! component: card This is the name of the generic UI component, not the React module! component: text field This is the name of the generic UI component, not the React module! design: joy This is about Joy Design, please involve a visual or UX designer in the process package: joy-ui Specific to @mui/joy
Projects
Status: Recently completed
Development

Successfully merging this pull request may close these issues.

5 participants