Feature/login and reset password form #114
Conversation
anetasliwinska
force-pushed
the
feature/login-and-reset-password-form
branch
3 times, most recently
from
55643ac to
b7815c8
Compare
…or reset password page
anetasliwinska
force-pushed
the
feature/login-and-reset-password-form
branch
from
da193cc to
a3d2d9e
Compare
…b.com/o2sdev/openselfservice into feature/login-and-reset-password-form
| </Typography> | ||
| <div className="flex flex-col gap-12 w-full"> | ||
| <div className="flex flex-col gap-6"> | ||
| {labels.resetPasswordMessage && ( |
There was a problem hiding this comment.
this needs another condition - labels should always be provided, but the message itself should be displayed based on an URL parameter, e.g. ?passwordReset=true
There was a problem hiding this comment.
Ok. I made a change.
| <div className="flex flex-col gap-12 w-full"> | ||
| <div className="flex flex-col gap-6"> | ||
| {labels.resetPasswordMessage && ( | ||
| <Banner |
| <Typography variant="small" className="mt-1"> | ||
| {labels.invalidCredentials} | ||
| {labels.newPasswordMessage && ( | ||
| <Banner |
| {validations && ( | ||
| <InputValidations | ||
| targetInputName={field.name} | ||
| validations={validations as InputValidationRegExpLabel[]} |
There was a problem hiding this comment.
could this casting be removed if you type it as validations: InputValidationRegExpLabel[]?
There was a problem hiding this comment.
Of course - I changed it.
| useEffect(() => { | ||
| onValid && onValid(active.every((el) => el)); | ||
| }, [active, onValid]); | ||
| return ( |
There was a problem hiding this comment.
code style - let's add an empty line between main parts of code (e.g. here before and after useEffect) for clarity
There was a problem hiding this comment.
Ok. Changed.
| return ( | ||
| <div className="grid grid-cols-2 gap-2"> | ||
| {validations.map(({ label }, index) => ( | ||
| <div className="flex items-center gap-2" key={`${label}-${index}`}> |
There was a problem hiding this comment.
maybe it could be a list?
There was a problem hiding this comment.
A good idea. I handled 2 situations - when map includes the only one element it is rendered in a <div> structure for more elements as a list
| @@ -6,7 +6,7 @@ export const AuthLayout: React.FC<AuthLayoutProps> = ({ layout = 'main-side', to | |||
| const main = ( | |||
| <div className="flex flex-col md:max-w-[50%] w-full p-4"> | |||
There was a problem hiding this comment.
perhaps the content should be centered vertically on mobile as well (it already is on desktop)? we could consult UX about this
There was a problem hiding this comment.
Very good idea - I made a change.
| import { FormValues, ResetPasswordFormProps } from './ResetPassword.types'; | ||
|
|
||
| export const ResetPasswordForm: React.FC<Readonly<ResetPasswordFormProps>> = ({ labels, onResetPassword }) => { | ||
| const [isSubmitting, setIsSubmitting] = useState(false); |
There was a problem hiding this comment.
this could be replaced with the new feature in react 19 https://react.dev/reference/react/useTransition (there are some examples of this in the app)
here and in other forms (sign-in, create new password)
There was a problem hiding this comment.
Added also to SignInForm and CreateNewPasswordForm. I also added LoadingOverlay.
| forgotPassword?: { | ||
| label: string; | ||
| link: string; | ||
| }; | ||
| resetPasswordMessage?: { | ||
| title?: string; | ||
| description?: string; | ||
| }; | ||
| newPasswordMessage?: { | ||
| title?: string; | ||
| description?: string; | ||
| }; |
There was a problem hiding this comment.
let's make these mandatory
Small UI changes in SingIn Form
Adding reset password form.