[SignInPage] Improve default UI and customisation ability#4370
[SignInPage] Improve default UI and customisation ability#4370bharatkashyap merged 19 commits intomui:masterfrom
Conversation
bharatkashyap
added
design: ui
apedroferreira
left a comment
apedroferreira
left a comment
There was a problem hiding this comment.
Both versions look really cool from the images!
In the alert in the themed examples, you could highlight the test email and password somehow (bold, italic, a color, whichever looks better, probably bold).
Are you still going to add a playground? Not sure if I should be reviewing yet.
Just for headerInfo I think we can come up with a better name, other than that all looks good.
| borderRadius: 1, | ||
| p: 4, | ||
| border: '1px solid', | ||
| borderColor: (theme) => alpha(theme.palette.grey[400], 0.4), |
There was a problem hiding this comment.
If you're using functions in properties more than once I guess the whole sx can be a function as the first argument should be the theme?
| * Custom information added to the top header beneath the title | ||
| * @default Typography | ||
| */ | ||
| headerInfo?: React.ElementType; |
There was a problem hiding this comment.
By header you mean the title, right? But we've been calling "header" to something more like the AppBar on top of pages. So maybe instead of "header" it could be "heading" or "title"?
There was a problem hiding this comment.
It's the section that could contain a sub-heading, given the heading of the Sign In Page is "Sign In", but I get your concern about the conflict with the "header" being the app bar. We could call it secondaryTitle?
There was a problem hiding this comment.
That sounds ok, on second thought maybe "headerInfo" isn't bad either but it could get confusing if we end up adding a header with the theme switcher I guess...
Or even something like "subHeading" could work probably. Feel free to go with what you think is best in the end, just thought I'd mention this.
There was a problem hiding this comment.
Yes, on second thought I could probably add two slots called title and subtitle to be able to override that entire section. What do you think?
There was a problem hiding this comment.
title and subtitle sound super clear to me.
| required | ||
| slotProps={{ | ||
| htmlInput: { | ||
| sx: (theme) => ({ |
There was a problem hiding this comment.
Also if we're using the theme so much, we could just call it from the useTheme hook?
…oolpad into feat/sign-in-page-ux
Added a playground for you to test. Run |
Wow it looks really good! Some feedback from what I tested:
Not sure if it should show the error in the app itself instead?
These don't need to be blocking as they're probably minor issues. |
Good catch. Fixing these!
It's planned to add an |
| "prettier": "pretty-quick --ignore-path .eslintignore", | ||
| "prettier:all": "prettier --write . --ignore-path .eslintignore", | ||
| "dev": "dotenv cross-env FORCE_COLOR=1 lerna -- run dev --stream --parallel --ignore docs --ignore playground-nextjs --ignore playground-nextjs-pages --ignore playground-vite --ignore playground-nextjs-themed", | ||
| "dev": "dotenv cross-env FORCE_COLOR=1 lerna -- run dev --stream --parallel --ignore docs --ignore playground-*", |
There was a problem hiding this comment.
Cool, didn't know you could do this!
3 participants
Closes [template] [ux]
SignInPageimprovements #4067Closes Expose sx for SignInPage/SignUpPage #4337
Closes [core]
DashboardLayouttheme overrides issues #4066Removes the default
LockIcon- users can display an icon usingbranding.logoAdds
titleandsubtitleslots toSignInPageto override the defaultsChanges the color of the
ListItemTextinside theDashboardLayoutNavigation totext.primarywhen selectedChanges to the default styling on the buttons and input fields
Adds an
sxprop to the outermost<Box />on theSignInPageAdds an
sxprop to theAccountPreviewcomponent applied to the wrapping<Stack />in bothexpandedandcondensedvariants