[SignInPage] Remove default `RememberMe` by bharatkashyap · Pull Request #4574 · mui/toolpad

bharatkashyap · 2024-12-30T10:37:39Z

Breaking Change

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

To Do:

we keep the rememeberMe
default: null
demo for users that are relying on the default ATM
mark as breaking change in release notes

mui-bot · 2024-12-30T10:53:36Z

Netlify deploy preview

https://deploy-preview-4574--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against c71d806

…lots-1

apedroferreira

Took a look and left some comments!
Overall the idea of the "remember me" slot being more generic and optional with its own component seems good to me.

apedroferreira · 2025-01-08T15:04:50Z

docs/data/toolpad/core/components/sign-in-page/SlotPropsSignIn.js

          passwordField: { variant: 'standard' },
          submitButton: { variant: 'outlined' },
-          rememberMe: {
+          checkbox: {


checkbox is more generic than rememberMe so I think it's an improvement, but could we make it more generic even as any type of content could go in this slot, such as a small disclaimer for example, or multiple checkboxes?
Not sure if something like formFooter would be a good name...
Anyway these are just some thoughts!

Also I see we have a forgotPasswordLink slot too, so if when implementing these you saw that there's a benefit to these slots being more specific then it should be fine.

Agreed, formFooter makes more sense!

Agreed, I did feel thatforgotPasswordLink and signUpLink seem hyper-specific. If we're offering the entire formFooter area as a slot, perhaps it makes sense to remove forgotPasswordLink entirely with this PR? If someone wants a forgot password link, they can simply add it in their slot component.

I would propose removing forgotPasswordLink, rememberMe and signUpLink to create this structure (including a new form slot to override the entire form):

Wdyt? Also @Janpot and @prakhargupta1

Might make sense to move this to the next release and mark that as having a lot of breaking changes for the SignInPage slots

Moving those to "plug-in" components seems good to me if they're complex enough!

apedroferreira · 2025-01-08T15:05:15Z

docs/data/toolpad/core/components/sign-in-page/SlotPropsSignIn.js

            `Signing in with "${provider.name}" and credentials: ${formData.get('email')}, ${formData.get('password')} and checkbox value: ${formData.get('tandc')}`,
          )
        }
+        slots={{ checkbox: RememberCheckbox }}


What about RememberMeCheckbox, would it be just a bit more descriptive?

docs/data/toolpad/core/components/sign-in-page/sign-in-page.md

docs/translations/api-docs/sign-in-page/sign-in-page.json

apedroferreira · 2025-01-08T15:09:33Z

packages/toolpad-core/src/SignInPage/RememberCheckbox.tsx

+          color: 'textSecondary',
+          fontSize: theme.typography.pxToRem(14),
+        },
+        ...props?.slotProps?.typography,


Shouldn't we just spread this inside typography instead?

apedroferreira · 2025-01-08T15:11:33Z

packages/toolpad-core/src/SignInPage/RememberCheckbox.tsx

+import { FormControlLabel, Checkbox } from '@mui/material';
+import { useTheme } from '@mui/material/styles';
+
+interface RememberCheckboxProps {


Can't we just allow all slotProps from FormControlLabel and make sure they're passed to the component?

Co-authored-by: Pedro Ferreira <[email protected]> Signed-off-by: Bharat Kashyap <[email protected]>

…lpad into enh/signin-slots-1

apedroferreira

Nice!

apedroferreira · 2025-01-24T18:47:53Z

packages/toolpad-core/src/SignInPage/RememberMeCheckbox.tsx

+      label="Remember me"
+      {...props}
+      control={
+        props.control ? (


Can just do props.control ??, right?

apedroferreira · 2025-01-24T18:49:05Z

packages/toolpad-core/src/SignInPage/SignInPage.tsx

                          label: 'Password',
                          id: 'password',
-                          placeholder: '*****',
+                          placeholder: '***** asdasd',


Suggested change

placeholder: '***** asdasd',

placeholder: '*****',

Testing leftovers? :D

😅 Thanks for the catch!

…lots-1

fix: Make rememberMe optional, add form slotProp

0171697

bharatkashyap added the component: sign-in label Dec 30, 2024

fix: CI

0cc237c

bharatkashyap added 3 commits December 30, 2024 16:26

fix: missed

e703c98

fix: Improvements

bf0042f

Merge branch 'master' into enh/signin-slots-1

a1b6be4

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 6, 2025

Merge branch 'master' of github.com:mui/mui-toolpad into enh/signin-s…

1ad8dad

…lots-1

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 7, 2025

bharatkashyap changed the title ~~[SignInPage] Make rememberMe optional, add form slotProp~~ [SignInPage] Make rememberMe optional Jan 7, 2025

bharatkashyap changed the title ~~[SignInPage] Make rememberMe optional~~ [SignInPage] [breaking] Rename rememberMe slot to checkbox Jan 7, 2025

bharatkashyap changed the title ~~[SignInPage] [breaking] Rename rememberMe slot to checkbox~~ [SignInPage] Rename rememberMe slot to checkbox Jan 7, 2025

bharatkashyap added 3 commits January 7, 2025 15:34

fix: Clearer and less confusing

253afec

fix: Proptypes

cc831be

Merge branch 'master' into enh/signin-slots-1

fe77c05

apedroferreira reviewed Jan 8, 2025

View reviewed changes

Update docs/data/toolpad/core/components/sign-in-page/sign-in-page.md

b28a2b1

Co-authored-by: Pedro Ferreira <[email protected]> Signed-off-by: Bharat Kashyap <[email protected]>

bharatkashyap changed the title ~~[SignInPage] Rename rememberMe slot to checkbox~~ [SignInPage] Revamp slots Jan 13, 2025

bharatkashyap mentioned this pull request Jan 13, 2025

Add new slot prop 'providerButton' to the SignInPage component #4584

Closed

bharatkashyap changed the title ~~[SignInPage] Revamp slots~~ [SignInPage] Improve slots Jan 24, 2025

bharatkashyap changed the title ~~[SignInPage] Improve slots~~ [SignInPage] Remove default RememberMe Jan 24, 2025

bharatkashyap added 3 commits January 24, 2025 02:30

Merge branch 'master' into enh/signin-slots-1

49f9dd4

fix: Fix scope creep

5e3ba71

Merge branch 'enh/signin-slots-1' of github.com:bharatkashyap/mui-too…

2095b43

…lpad into enh/signin-slots-1

bharatkashyap requested a review from apedroferreira January 24, 2025 11:09

fix: Missed

67bd6f8

apedroferreira approved these changes Jan 24, 2025

View reviewed changes

bharatkashyap added 2 commits January 27, 2025 14:48

fix: Pedro review

d1ca129

fix: CI

17316b5

bharatkashyap changed the title ~~[SignInPage] Remove default RememberMe~~ [SignInPage] Remove default RememberMe and add oauthButton Jan 27, 2025

bharatkashyap changed the title ~~[SignInPage] Remove default RememberMe and add oauthButton~~ [SignInPage] Remove default RememberMe and add oauthButton slotProp Jan 27, 2025

bharatkashyap added 2 commits January 27, 2025 15:33

fix: Add oauthButton slot prop to demo

0054e33

fix: Lint

b8bfcde

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 3, 2025

fix: Minimise breaking changes

5d1fe9e

bharatkashyap changed the title ~~[SignInPage] Remove default RememberMe and add oauthButton slotProp~~ [SignInPage] Remove default RememberMe Feb 12, 2025

bharatkashyap added a commit to bharatkashyap/mui-toolpad that referenced this pull request Feb 12, 2025

fix: Move out of mui#4574

223fa80

bharatkashyap mentioned this pull request Feb 12, 2025

[feat] Add form and oauthButton slot props #4674

Merged

bharatkashyap added 2 commits April 4, 2025 12:27

Merge branch 'master' of github.com:mui/mui-toolpad into enh/signin-s…

1eb0755

…lots-1

fix: CI

c71d806

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 4, 2025

bharatkashyap requested a review from Janpot April 8, 2025 18:15

bharatkashyap added this to Toolpad public roadmap Apr 11, 2025

bharatkashyap moved this to In progress in Toolpad public roadmap Apr 11, 2025

Janpot assigned bharatkashyap Apr 14, 2025

bharatkashyap requested a review from apedroferreira April 16, 2025 21:04

apedroferreira approved these changes Apr 23, 2025

View reviewed changes

bharatkashyap merged commit 6d4f23f into mui:master Apr 23, 2025
15 checks passed

github-project-automation bot moved this from In progress to Completed in Toolpad public roadmap Apr 23, 2025

zannager added the scope: toolpad-core Abbreviated to "core" label Sep 9, 2025

Uh oh!

Conversation

bharatkashyap commented Dec 30, 2024 • edited by Janpot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking Change

Uh oh!

mui-bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Netlify deploy preview

Uh oh!

apedroferreira left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bharatkashyap Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apedroferreira left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bharatkashyap commented Dec 30, 2024 •

edited by Janpot

Loading

mui-bot commented Dec 30, 2024 •

edited

Loading

bharatkashyap Jan 9, 2025 •

edited

Loading