From f89a4406a5f545c690940ead376a9a608e84e7c9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 17 Nov 2023 18:08:34 +0000 Subject: [PATCH 1/2] refactor(site): refactor login screen --- .vscode/settings.json | 1 + site/src/@types/mui.d.ts | 4 ++ site/src/pages/LoginPage/LoginPageView.tsx | 2 +- site/src/pages/LoginPage/OAuthSignInForm.tsx | 63 +++++++++---------- .../pages/LoginPage/PasswordSignInForm.tsx | 48 +++++++------- .../pages/LoginPage/SignInForm.stories.tsx | 3 - site/src/pages/LoginPage/SignInForm.tsx | 54 ++++------------ site/src/pages/LoginPage/SignInForm.types.ts | 9 --- site/src/theme/constants.ts | 1 + site/src/theme/mui.ts | 10 ++- 10 files changed, 77 insertions(+), 118 deletions(-) delete mode 100644 site/src/pages/LoginPage/SignInForm.types.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index ce4c0abe51839..890e561520ff7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -170,6 +170,7 @@ "wsconncache", "wsjson", "xerrors", + "xlarge", "yamux" ], "cSpell.ignorePaths": ["site/package.json", ".vscode/settings.json"], diff --git a/site/src/@types/mui.d.ts b/site/src/@types/mui.d.ts index 4cba286dc6dee..0762f53151c59 100644 --- a/site/src/@types/mui.d.ts +++ b/site/src/@types/mui.d.ts @@ -31,6 +31,10 @@ declare module "@mui/material/Button" { interface ButtonPropsColorOverrides { neutral: true; } + + interface ButtonPropsSizeOverrides { + xlarge: true; + } } declare module "@mui/system" { diff --git a/site/src/pages/LoginPage/LoginPageView.tsx b/site/src/pages/LoginPage/LoginPageView.tsx index 6866ef25620dd..4b7f310ddebef 100644 --- a/site/src/pages/LoginPage/LoginPageView.tsx +++ b/site/src/pages/LoginPage/LoginPageView.tsx @@ -75,7 +75,7 @@ const styles = { container: { width: "100%", - maxWidth: 385, + maxWidth: 320, display: "flex", flexDirection: "column", alignItems: "center", diff --git a/site/src/pages/LoginPage/OAuthSignInForm.tsx b/site/src/pages/LoginPage/OAuthSignInForm.tsx index 2fd38bb441e42..6c75c5ec74cff 100644 --- a/site/src/pages/LoginPage/OAuthSignInForm.tsx +++ b/site/src/pages/LoginPage/OAuthSignInForm.tsx @@ -1,4 +1,3 @@ -import Link from "@mui/material/Link"; import Button from "@mui/material/Button"; import GitHubIcon from "@mui/icons-material/GitHub"; import KeyIcon from "@mui/icons-material/VpnKey"; @@ -26,49 +25,47 @@ export const OAuthSignInForm: FC = ({ return ( {authMethods?.github.enabled && ( - } + disabled={isSigningIn} + fullWidth + type="submit" + size="xlarge" > - - + {Language.githubSignIn} + )} {authMethods?.oidc.enabled && ( - + ) : ( + + ) + } + disabled={isSigningIn} + fullWidth + type="submit" > - - + {authMethods.oidc.signInText || Language.oidcSignIn} + )} ); diff --git a/site/src/pages/LoginPage/PasswordSignInForm.tsx b/site/src/pages/LoginPage/PasswordSignInForm.tsx index fe3252bfa1ccc..eb60407f5a305 100644 --- a/site/src/pages/LoginPage/PasswordSignInForm.tsx +++ b/site/src/pages/LoginPage/PasswordSignInForm.tsx @@ -2,22 +2,21 @@ import { Stack } from "components/Stack/Stack"; import TextField from "@mui/material/TextField"; import { getFormHelpers, onChangeTrimmed } from "utils/formUtils"; import { Language } from "./SignInForm"; -import { FormikContextType, FormikTouched, useFormik } from "formik"; +import { useFormik } from "formik"; import * as Yup from "yup"; import { FC } from "react"; -import { BuiltInAuthFormValues } from "./SignInForm.types"; import LoadingButton from "@mui/lab/LoadingButton"; type PasswordSignInFormProps = { onSubmit: (credentials: { email: string; password: string }) => void; - initialTouched?: FormikTouched; isSigningIn: boolean; + autoFocus: boolean; }; export const PasswordSignInForm: FC = ({ onSubmit, - initialTouched, isSigningIn, + autoFocus, }) => { const validationSchema = Yup.object({ email: Yup.string() @@ -27,17 +26,16 @@ export const PasswordSignInForm: FC = ({ password: Yup.string(), }); - const form: FormikContextType = - useFormik({ - initialValues: { - email: "", - password: "", - }, - validationSchema, - onSubmit, - initialTouched, - }); - const getFieldHelpers = getFormHelpers(form); + const form = useFormik({ + initialValues: { + email: "", + password: "", + }, + validationSchema, + onSubmit, + validateOnBlur: false, + }); + const getFieldHelpers = getFormHelpers(form); return (
@@ -45,7 +43,7 @@ export const PasswordSignInForm: FC = ({ = ({ label={Language.passwordLabel} type="password" /> -
- - {Language.passwordSignIn} - -
+ + {Language.passwordSignIn} + ); diff --git a/site/src/pages/LoginPage/SignInForm.stories.tsx b/site/src/pages/LoginPage/SignInForm.stories.tsx index 7408eaa97c04d..28c1305ad11d4 100644 --- a/site/src/pages/LoginPage/SignInForm.stories.tsx +++ b/site/src/pages/LoginPage/SignInForm.stories.tsx @@ -37,9 +37,6 @@ export const WithError: Story = { }, ], }), - initialTouched: { - password: true, - }, }, }; diff --git a/site/src/pages/LoginPage/SignInForm.tsx b/site/src/pages/LoginPage/SignInForm.tsx index 10e915b441d07..fe48fd397c9b6 100644 --- a/site/src/pages/LoginPage/SignInForm.tsx +++ b/site/src/pages/LoginPage/SignInForm.tsx @@ -1,15 +1,10 @@ import { type Interpolation, type Theme } from "@emotion/react"; -import { type FormikTouched } from "formik"; -import { type FC, useState } from "react"; +import { type FC } from "react"; import type { AuthMethods } from "api/typesGenerated"; import { PasswordSignInForm } from "./PasswordSignInForm"; import { OAuthSignInForm } from "./OAuthSignInForm"; -import { type BuiltInAuthFormValues } from "./SignInForm.types"; -import Button from "@mui/material/Button"; -import EmailIcon from "@mui/icons-material/EmailOutlined"; import { Alert } from "components/Alert/Alert"; import { ErrorAlert } from "components/Alert/ErrorAlert"; -import { getApplicationName } from "utils/appearance"; export const Language = { emailLabel: "Email", @@ -71,8 +66,6 @@ export interface SignInFormProps { info?: string; authMethods?: AuthMethods; onSubmit: (credentials: { email: string; password: string }) => void; - // initialTouched is only used for testing the error state of the form. - initialTouched?: FormikTouched; } export const SignInForm: FC> = ({ @@ -82,21 +75,15 @@ export const SignInForm: FC> = ({ error, info, onSubmit, - initialTouched, }) => { const oAuthEnabled = Boolean( authMethods?.github.enabled || authMethods?.oidc.enabled, ); const passwordEnabled = authMethods?.password.enabled ?? true; - // Hide password auth by default if any OAuth method is enabled - const [showPasswordAuth, setShowPasswordAuth] = useState(!oAuthEnabled); - const applicationName = getApplicationName(); return (
-

- Sign in to {applicationName} -

+

Sign in

{Boolean(error) && (
@@ -110,15 +97,15 @@ export const SignInForm: FC> = ({
)} - {passwordEnabled && showPasswordAuth && ( - )} - {passwordEnabled && showPasswordAuth && oAuthEnabled && ( + {passwordEnabled && oAuthEnabled && (
Or
@@ -126,36 +113,17 @@ export const SignInForm: FC> = ({
)} - {oAuthEnabled && ( - )} {!passwordEnabled && !oAuthEnabled && ( No authentication methods configured! )} - - {passwordEnabled && !showPasswordAuth && ( - <> -
-
-
Or
-
-
- - - - )}
); }; diff --git a/site/src/pages/LoginPage/SignInForm.types.ts b/site/src/pages/LoginPage/SignInForm.types.ts deleted file mode 100644 index a143b46e4d27a..0000000000000 --- a/site/src/pages/LoginPage/SignInForm.types.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** - * BuiltInAuthFormValues describes a form using built-in (email/password) - * authentication. This form may not always be present depending on external - * auth providers available and administrative configurations - */ -export interface BuiltInAuthFormValues { - email: string; - password: string; -} diff --git a/site/src/theme/constants.ts b/site/src/theme/constants.ts index 32282a1ad73a8..5c58eed56a697 100644 --- a/site/src/theme/constants.ts +++ b/site/src/theme/constants.ts @@ -9,6 +9,7 @@ export const sidePadding = 24; export const dashboardContentBottomPadding = 8 * 6; // MUI does not have aligned heights for buttons and inputs so we have to "hack" it a little bit +export const BUTTON_XL_HEIGHT = 44; export const BUTTON_LG_HEIGHT = 40; export const BUTTON_MD_HEIGHT = 36; export const BUTTON_SM_HEIGHT = 32; diff --git a/site/src/theme/mui.ts b/site/src/theme/mui.ts index f54c4e6691bec..86f0d26019274 100644 --- a/site/src/theme/mui.ts +++ b/site/src/theme/mui.ts @@ -6,6 +6,7 @@ import { BUTTON_LG_HEIGHT, BUTTON_MD_HEIGHT, BUTTON_SM_HEIGHT, + BUTTON_XL_HEIGHT, } from "./constants"; // eslint-disable-next-line no-restricted-imports -- We need MUI here import { alertClasses } from "@mui/material/Alert"; @@ -172,6 +173,9 @@ dark = createTheme(dark, { sizeLarge: { height: BUTTON_LG_HEIGHT, }, + sizeXlarge: { + height: BUTTON_XL_HEIGHT, + }, outlined: { ":hover": { border: `1px solid ${colors.gray[11]}`, @@ -190,10 +194,10 @@ dark = createTheme(dark, { }, }, containedNeutral: { - borderColor: colors.gray[12], - backgroundColor: colors.gray[13], + backgroundColor: colors.gray[14], + "&:hover": { - backgroundColor: colors.gray[12], + backgroundColor: colors.gray[13], }, }, iconSizeMedium: { From 3900c4a730a238f7800c1d5172ea9f65f1a4bb1d Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 17 Nov 2023 18:52:10 +0000 Subject: [PATCH 2/2] Fix test --- site/src/pages/LoginPage/LoginPage.test.tsx | 29 --------------------- 1 file changed, 29 deletions(-) diff --git a/site/src/pages/LoginPage/LoginPage.test.tsx b/site/src/pages/LoginPage/LoginPage.test.tsx index 1fb78f9f196c2..3ca2b26a73d08 100644 --- a/site/src/pages/LoginPage/LoginPage.test.tsx +++ b/site/src/pages/LoginPage/LoginPage.test.tsx @@ -10,7 +10,6 @@ import { } from "testHelpers/renderHelpers"; import { server } from "testHelpers/server"; import { LoginPage } from "./LoginPage"; -import * as TypesGen from "api/typesGenerated"; describe("LoginPage", () => { beforeEach(() => { @@ -76,32 +75,4 @@ describe("LoginPage", () => { // Then await screen.findByText("Setup"); }); - - it("hides password authentication if OIDC/GitHub is enabled and displays on click", async () => { - const authMethods: TypesGen.AuthMethods = { - password: { enabled: true }, - github: { enabled: true }, - oidc: { enabled: true, signInText: "", iconUrl: "" }, - }; - - // Given - server.use( - rest.get("/api/v2/users/authmethods", async (req, res, ctx) => { - return res(ctx.status(200), ctx.json(authMethods)); - }), - ); - - // When - render(); - - // Then - expect(screen.queryByText(Language.passwordSignIn)).not.toBeInTheDocument(); - await screen.findByText(Language.githubSignIn); - - const showPasswordAuthLink = screen.getByText("Email and password"); - await userEvent.click(showPasswordAuthLink); - - await screen.findByText(Language.passwordSignIn); - await screen.findByText(Language.githubSignIn); - }); });