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][joy-ui] Refine the Profile Dashboard template #38599

Merged
merged 26 commits into from
Sep 5, 2023

Conversation

zanivan
Copy link
Contributor

@zanivan zanivan commented Aug 22, 2023

Refining the Order Dashboard template aiming to remove most of the custom styles and overrides, making it closer as possible to the default theme.

Part of #38582

https://deploy-preview-38599--material-ui.netlify.app/joy-ui/getting-started/templates/profile-dashboard/

@zanivan zanivan added docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy design: joy This is about Joy Design, please involve a visual or UX designer in the process labels Aug 22, 2023
@zanivan zanivan self-assigned this Aug 22, 2023
@zanivan zanivan mentioned this pull request Aug 22, 2023
10 tasks
@mui-bot
Copy link

mui-bot commented Aug 22, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 5f45279

@zanivan zanivan marked this pull request as ready for review September 1, 2023 18:19
@zanivan
Copy link
Contributor Author

zanivan commented Sep 1, 2023

@siriwatknp this was an old template inspired by an asset from Untitled UI kit, but I don't know if this still makes sense. Should we remove the reference now that's revamped and looks way different from before?

@Studio384
Copy link
Contributor

Studio384 commented Sep 4, 2023

The top and bottom spacing for the CardActions are different, and since they have a colored background, they appear to be misaligned.

@zanivan
Copy link
Contributor Author

zanivan commented Sep 5, 2023

@siriwatknp are there any specific reason to replace the Accordion on the 32a9687?

@siriwatknp
Copy link
Member

siriwatknp commented Sep 5, 2023

@siriwatknp are there any specific reason to replace the Accordion on the 32a9687?

Mainly, accessibility. The navigation should be nav > ul > li > a, not accordion. The user with a screen reader will be misled if the accordion is used.

dangerfile.ts Outdated
@@ -1,7 +1,7 @@
// inspire by reacts dangerfile
// danger has to be the first thing required!
import { danger, markdown } from 'danger';
import { exec } from 'child_process';
Copy link
Member

Choose a reason for hiding this comment

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

@zanivan I think this change should be reverted.

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.

@zanivan Great job! I really like the new look of the template, especially the shadow of the form controls.

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, yet another sweet improvement to the existing templates! Many of the things I had noticed the last time I saw it a few days ago were fixed already, so I'm left with just a couple of tiny stuff, none of which are blockers for moving forward, I think:

  1. The profile tabs (e.g., "Settings", etc.) seem to be missing the hover state. We could also round the top corners a bit.
  2. This might be on the component level (and not on the template), but inputs also seem to be missing hover styles. Like, I find it a bit weird that the Timezone select has and everything else doesn't. To verify if that's already correct and just not working here for some reason (similarly above).
  3. I think we could add a top divider on the gray container holding the "Cancel" and "Save" buttons! Just a bit of polish.

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

zanivan commented Sep 5, 2023

The profile tabs (e.g., "Settings", etc.) seem to be missing the hover state. We could also round the top corners a bit.

I don't know if it's a bug, but the only tab without a hover state is the selected one

This might be on the component level (and not on the template), but inputs also seem to be missing hover styles [...]

This seems to be an issue on the component level. The select has a hover state, but the input, autocomplete, and the other doesn't. IMO, this should be tackled in a different PR if so.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 5, 2023
@zanivan zanivan merged commit d9cdb05 into mui:master Sep 5, 2023
5 checks passed
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
xcode-it pushed a commit to xcode-it/material-ui that referenced this pull request Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design: joy This is about Joy Design, please involve a visual or UX designer in the process docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy
Projects
Status: Recently completed
Development

Successfully merging this pull request may close these issues.

5 participants