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

Provide an option for SignInPage to skip the stack with "Remember Me" and "Forgot Password" #4572

Open
kirill-konshin opened this issue Dec 29, 2024 · 6 comments · May be fixed by #4574
Open
Labels
enhancement This is not a bug, nor a new feature

Comments

@kirill-konshin
Copy link

kirill-konshin commented Dec 29, 2024

Summary

Currently <SignInPage slots={{ rememberMe: () => null }} /> results in an empty stack:

image

And a gap between field and button:

image

Also note the padding on "Password" is off by few pixels, label is shifted down.

Examples

Something like <SignInPage slots={{ rememberMe: null }} /> and not providing the forgotPasswordLink.

Currently it's not allowed:

image

Motivation

There are cases when users should not be presented neither with "Remember Me" nor "Forgot Password". The stack is unconditional now:

image

Search keywords: SignInPage Stack

@kirill-konshin kirill-konshin added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 29, 2024
@kirill-konshin
Copy link
Author

P.S. Allow to pass custom props to form: < SignInPage slotProps: {form: {noValidate: true}}> to disable client side browser-default validation:

image

@bharatkashyap bharatkashyap added enhancement This is not a bug, nor a new feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 30, 2024
@bharatkashyap
Copy link
Member

bharatkashyap commented Dec 30, 2024

Sounds good. I feel like the "Remember Me" checkbox shouldn't be there by default at all - instead, we can export a Remember component from SignInPage and allow users to pass that into the rememberMe slot if they need it.
Wdyt @mui/toolpad?

@kirill-konshin
Copy link
Author

kirill-konshin commented Dec 30, 2024

@bharatkashyap I'd agree. It would be much simpler to use the component as a composite with slots, instead of configuration based. Some slots can be a bit more general purpose, e.g. "area between fields and submit". Btw, "subtitle" and "title" seem to be swapped ("subtitle" is "Sign In" now), should be the other way.

Btw, the gap is there even if I set "display: none" for the Stack, margin-bottom of text field adds up with margin-top of button.

@bharatkashyap
Copy link
Member

Btw, "subtitle" and "title" seem to be swapped ("subtitle" is "Sign In" now), should be the other way.

I can't seem to catch where this issue is

Btw, the gap is there even if I set "display: none" for the Stack, margin-bottom of text field adds up with margin-top of button.

Yes, the text field margin-bottom would also need to go

@kirill-konshin
Copy link
Author

kirill-konshin commented Dec 31, 2024

Btw, "subtitle" and "title" seem to be swapped ("subtitle" is "Sign In" now), should be the other way.

I can't seem to catch where this issue is

Looks like I messed up during my experiments, I can't reproduce it now, please disregard.

@kirill-konshin
Copy link
Author

kirill-konshin commented Dec 31, 2024

Btw, "subtitle" and "title" seem to be swapped ("subtitle" is "Sign In" now), should be the other way.

I can't seem to catch where this issue is

Btw, the gap is there even if I set "display: none" for the Stack, margin-bottom of text field adds up with margin-top of button.

Yes, the text field margin-bottom would also need to go

You can have a Stack instead of the Box, this way simple gap will give equal spacing between form and the rest (and between titles too), and form itself is a Stack anyway.

image

Or titles and other things can just reside inside the form, less markup this way. I usually make the whole Card a form and CardContent a Stack, this way I get correct spacing and can put buttons in CardFooter. It should be semantically correct too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants