diff --git a/.betterer.results b/.betterer.results index 50c4ffb3006..7f1610b1ff5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1139,9 +1139,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"] ], - "public/app/core/components/ForgottenPassword/ChangePassword.tsx:5381": [ - [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] - ], "public/app/core/components/ForgottenPassword/ForgottenPassword.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"] ], @@ -1178,38 +1175,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "3"], [0, 0, 0, "Styles should be written using objects.", "4"] ], - "public/app/core/components/Login/LoginForm.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Styles should be written using objects.", "1"], - [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "2"], - [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "3"] - ], - "public/app/core/components/Login/LoginLayout.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Styles should be written using objects.", "1"], - [0, 0, 0, "Styles should be written using objects.", "2"], - [0, 0, 0, "Styles should be written using objects.", "3"], - [0, 0, 0, "Styles should be written using objects.", "4"], - [0, 0, 0, "Styles should be written using objects.", "5"], - [0, 0, 0, "Styles should be written using objects.", "6"], - [0, 0, 0, "Styles should be written using objects.", "7"], - [0, 0, 0, "Styles should be written using objects.", "8"], - [0, 0, 0, "Styles should be written using objects.", "9"], - [0, 0, 0, "Styles should be written using objects.", "10"] - ], - "public/app/core/components/Login/LoginPage.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"] - ], - "public/app/core/components/Login/LoginServiceButtons.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Styles should be written using objects.", "1"], - [0, 0, 0, "Styles should be written using objects.", "2"], - [0, 0, 0, "Styles should be written using objects.", "3"], - [0, 0, 0, "Styles should be written using objects.", "4"] - ], - "public/app/core/components/Login/UserSignup.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"] - ], "public/app/core/components/NestedFolderPicker/Trigger.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"] ], @@ -1331,9 +1296,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"] ], - "public/app/core/components/PasswordField/PasswordField.tsx:5381": [ - [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] - ], "public/app/core/components/QueryOperationRow/OperationRowHelp.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"] ], diff --git a/.pa11yci-pr.conf.js b/.pa11yci-pr.conf.js index 71990e6a115..326925d0495 100644 --- a/.pa11yci-pr.conf.js +++ b/.pa11yci-pr.conf.js @@ -72,8 +72,8 @@ var config = { "wait for element input[name='user'] to be added", "set field input[name='user'] to admin", "set field input[name='password'] to admin", - "click element button[aria-label='Login button']", - "wait for element [aria-label='Skip change password button'] to be visible", + "click element button[data-testid='data-testid Login button']", + "wait for element button[data-testid='data-testid Skip change password button'] to be visible", ], threshold: 15, rootElement: '.main-view', diff --git a/.pa11yci.conf.js b/.pa11yci.conf.js index b15b7c7d80a..5c8256c3f3e 100644 --- a/.pa11yci.conf.js +++ b/.pa11yci.conf.js @@ -61,8 +61,8 @@ var config = { "wait for element input[name='user'] to be added", "set field input[name='user'] to admin", "set field input[name='password'] to admin", - "click element button[aria-label='Login button']", - "wait for element [aria-label='Skip change password button'] to be visible", + "click element button[data-testid='data-testid Login button']", + "wait for element button[data-testid='data-testid Skip change password button'] to be visible", ], wait: 500, rootElement: '.main-view', diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 65d92211882..e73278d243e 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -8,10 +8,10 @@ import { Components } from './components'; export const Pages = { Login: { url: '/login', - username: 'Username input field', - password: 'Password input field', - submit: 'Login button', - skip: 'Skip change password button', + username: 'data-testid Username input field', + password: 'data-testid Password input field', + submit: 'data-testid Login button', + skip: 'data-testid Skip change password button', }, Home: { url: '/', diff --git a/public/app/core/components/ForgottenPassword/ChangePassword.tsx b/public/app/core/components/ForgottenPassword/ChangePassword.tsx index d24442a91e1..989ab9f81b2 100644 --- a/public/app/core/components/ForgottenPassword/ChangePassword.tsx +++ b/public/app/core/components/ForgottenPassword/ChangePassword.tsx @@ -1,9 +1,9 @@ import React, { SyntheticEvent } from 'react'; import { selectors } from '@grafana/e2e-selectors'; -import { Tooltip, Form, Field, VerticalGroup, Button, Alert } from '@grafana/ui'; +import { Tooltip, Form, Field, VerticalGroup, Button, Alert, useStyles2 } from '@grafana/ui'; -import { submitButton } from '../Login/LoginForm'; +import { getStyles } from '../Login/LoginForm'; import { PasswordField } from '../PasswordField/PasswordField'; interface Props { onSubmit: (pw: string) => void; @@ -17,6 +17,7 @@ interface PasswordDTO { } export const ChangePassword = ({ onSubmit, onSkip, showDefaultPasswordWarning }: Props) => { + const styles = useStyles2(getStyles); const submit = (passwords: PasswordDTO) => { onSubmit(passwords.newPassword); }; @@ -29,24 +30,24 @@ export const ChangePassword = ({ onSubmit, onSkip, showDefaultPasswordWarning }: )} v === getValues().newPassword || 'Passwords must match!', })} + id="confirm-new-password" + autoComplete="new-password" /> - @@ -55,7 +56,7 @@ export const ChangePassword = ({ onSubmit, onSkip, showDefaultPasswordWarning }: content="If you skip you will be prompted to change password next time you log in." placement="bottom" > - diff --git a/public/app/core/components/Login/LoginForm.tsx b/public/app/core/components/Login/LoginForm.tsx index 45cdf8fa087..66ceb149afe 100644 --- a/public/app/core/components/Login/LoginForm.tsx +++ b/public/app/core/components/Login/LoginForm.tsx @@ -1,8 +1,9 @@ import { css } from '@emotion/css'; -import React, { ReactElement } from 'react'; +import React, { ReactElement, useId } from 'react'; +import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { Button, Form, Input, Field } from '@grafana/ui'; +import { Button, Form, Input, Field, useStyles2 } from '@grafana/ui'; import { PasswordField } from '../PasswordField/PasswordField'; @@ -16,43 +17,38 @@ interface Props { loginHint: string; } -const wrapperStyles = css` - width: 100%; - padding-bottom: 16px; -`; - -export const submitButton = css` - justify-content: center; - width: 100%; -`; - export const LoginForm = ({ children, onSubmit, isLoggingIn, passwordHint, loginHint }: Props) => { + const styles = useStyles2(getStyles); + const usernameId = useId(); + const passwordId = useId(); + return ( -
+
{({ register, errors }) => ( <>
); }; + +export const getStyles = (theme: GrafanaTheme2) => { + return { + wrapper: css({ + width: '100%', + paddingBottom: theme.spacing(2), + }), + + submitButton: css({ + justifyContent: 'center', + width: '100%', + }), + }; +}; diff --git a/public/app/core/components/Login/LoginLayout.tsx b/public/app/core/components/Login/LoginLayout.tsx index 1488cb47d8f..33321102d73 100644 --- a/public/app/core/components/Login/LoginLayout.tsx +++ b/public/app/core/components/Login/LoginLayout.tsx @@ -2,7 +2,7 @@ import { cx, css, keyframes } from '@emotion/css'; import React, { useEffect, useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; -import { useStyles2, styleMixins } from '@grafana/ui'; +import { useStyles2 } from '@grafana/ui'; import { Branding } from '../Branding/Branding'; import { BrandingSettings } from '../Branding/types'; @@ -92,90 +92,90 @@ export const getLoginStyles = (theme: GrafanaTheme2) => { alignItems: 'center', justifyContent: 'center', }), - loginAnim: css` - &:before { - opacity: 1; - } + loginAnim: css({ + ['&:before']: { + opacity: 1, + }, - .login-content-box { - opacity: 1; - } - `, - submitButton: css` - justify-content: center; - width: 100%; - `, - loginLogo: css` - width: 100%; - max-width: 60px; - margin-bottom: 15px; + ['.login-content-box']: { + opacity: 1, + }, + }), + submitButton: css({ + justifyContent: 'center', + width: '100%', + }), + loginLogo: css({ + width: '100%', + maxWidth: 60, + marginBottom: theme.spacing(2), - @media ${styleMixins.mediaUp(theme.v1.breakpoints.sm)} { - max-width: 100px; - } - `, - loginLogoWrapper: css` - display: flex; - align-items: center; - justify-content: center; - flex-direction: column; - padding: ${theme.spacing(3)}; - `, - titleWrapper: css` - text-align: center; - `, - mainTitle: css` - font-size: 22px; + [theme.breakpoints.up('sm')]: { + maxWidth: 100, + }, + }), + loginLogoWrapper: css({ + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + flexDirection: 'column', + padding: theme.spacing(3), + }), + titleWrapper: css({ + textAlign: 'center', + }), + mainTitle: css({ + fontSize: 22, - @media ${styleMixins.mediaUp(theme.v1.breakpoints.sm)} { - font-size: 32px; - } - `, - subTitle: css` - font-size: ${theme.typography.size.md}; - color: ${theme.colors.text.secondary}; - `, - loginContent: css` - max-width: 478px; - width: calc(100% - 2rem); - display: flex; - align-items: stretch; - flex-direction: column; - position: relative; - justify-content: flex-start; - z-index: 1; - min-height: 320px; - border-radius: ${theme.shape.borderRadius(4)}; - padding: ${theme.spacing(2, 0)}; - opacity: 0; - transition: opacity 0.5s ease-in-out; + [theme.breakpoints.up('sm')]: { + fontSize: 32, + }, + }), + subTitle: css({ + fontSize: theme.typography.size.md, + color: theme.colors.text.secondary, + }), + loginContent: css({ + maxWidth: 478, + width: `calc(100% - 2rem)`, + display: 'flex', + alignItems: 'stretch', + flexDirection: 'column', + position: 'relative', + justifyContent: 'flex-start', + zIndex: 1, + minHeight: 320, + borderRadius: theme.shape.borderRadius(4), + padding: theme.spacing(2, 0), + opacity: 0, + transition: 'opacity 0.5s ease-in-out', - @media ${styleMixins.mediaUp(theme.v1.breakpoints.sm)} { - min-height: 320px; - justify-content: center; - } - `, - loginOuterBox: css` - display: flex; - overflow-y: hidden; - align-items: center; - justify-content: center; - `, - loginInnerBox: css` - padding: ${theme.spacing(0, 2, 2, 2)}; + [theme.breakpoints.up('sm')]: { + minHeight: theme.spacing(40), + justifyContent: 'center', + }, + }), + loginOuterBox: css({ + display: 'flex', + overflowU: 'hidden', + alignItems: 'center', + justifyContent: 'center', + }), + loginInnerBox: css({ + padding: theme.spacing(0, 2, 2, 2), - display: flex; - flex-direction: column; - align-items: center; - justify-content: center; - flex-grow: 1; - max-width: 415px; - width: 100%; - transform: translate(0px, 0px); - transition: 0.25s ease; - `, - enterAnimation: css` - animation: ${flyInAnimation} ease-out 0.2s; - `, + display: 'flex', + flexDirection: 'column', + alignItems: 'center', + justifyContent: 'center', + flexGrow: 1, + maxWidth: 415, + width: '100%', + transform: 'translate(0px, 0px)', + transition: '0.25s ease', + }), + enterAnimation: css({ + animation: `${flyInAnimation} ease-out 0.2s`, + }), }; }; diff --git a/public/app/core/components/Login/LoginPage.test.tsx b/public/app/core/components/Login/LoginPage.test.tsx index fba3892af2b..7a647e35f56 100644 --- a/public/app/core/components/Login/LoginPage.test.tsx +++ b/public/app/core/components/Login/LoginPage.test.tsx @@ -39,9 +39,9 @@ describe('Login Page', () => { render(); expect(screen.getByRole('heading', { name: 'Welcome to Grafana' })).toBeInTheDocument(); - expect(screen.getByRole('textbox', { name: 'Username input field' })).toBeInTheDocument(); - expect(screen.getByLabelText('Password input field')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Login button' })).toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: 'Email or username' })).toBeInTheDocument(); + expect(screen.getByLabelText('Password')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Log in' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Forgot your password?' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Forgot your password?' })).toHaveAttribute( @@ -56,20 +56,20 @@ describe('Login Page', () => { it('should pass validation checks for username field', async () => { render(); - fireEvent.click(screen.getByRole('button', { name: 'Login button' })); + fireEvent.click(screen.getByRole('button', { name: 'Log in' })); expect(await screen.findByText('Email or username is required')).toBeInTheDocument(); - await userEvent.type(screen.getByRole('textbox', { name: 'Username input field' }), 'admin'); + await userEvent.type(screen.getByRole('textbox', { name: 'Email or username' }), 'admin'); await waitFor(() => expect(screen.queryByText('Email or username is required')).not.toBeInTheDocument()); }); it('should pass validation checks for password field', async () => { render(); - fireEvent.click(screen.getByRole('button', { name: 'Login button' })); + fireEvent.click(screen.getByRole('button', { name: 'Log in' })); expect(await screen.findByText('Password is required')).toBeInTheDocument(); - await userEvent.type(screen.getByLabelText('Password input field'), 'admin'); + await userEvent.type(screen.getByLabelText('Password'), 'admin'); await waitFor(() => expect(screen.queryByText('Password is required')).not.toBeInTheDocument()); }); @@ -82,9 +82,9 @@ describe('Login Page', () => { postMock.mockResolvedValueOnce({ message: 'Logged in' }); render(); - await userEvent.type(screen.getByLabelText('Username input field'), 'admin'); - await userEvent.type(screen.getByLabelText('Password input field'), 'test'); - fireEvent.click(screen.getByLabelText('Login button')); + await userEvent.type(screen.getByLabelText('Email or username'), 'admin'); + await userEvent.type(screen.getByLabelText('Password'), 'test'); + fireEvent.click(screen.getByRole('button', { name: 'Log in' })); await waitFor(() => expect(postMock).toHaveBeenCalledWith('/login', { password: 'test', user: 'admin' }, { showErrorAlert: false }) @@ -128,9 +128,9 @@ describe('Login Page', () => { render(); - await userEvent.type(screen.getByLabelText('Username input field'), 'admin'); - await userEvent.type(screen.getByLabelText('Password input field'), 'test'); - await userEvent.click(screen.getByRole('button', { name: 'Login button' })); + await userEvent.type(screen.getByLabelText('Email or username'), 'admin'); + await userEvent.type(screen.getByLabelText('Password'), 'test'); + await userEvent.click(screen.getByRole('button', { name: 'Log in' })); const alert = await screen.findByRole('alert', { name: 'Login failed' }); expect(alert).toBeInTheDocument(); @@ -150,9 +150,9 @@ describe('Login Page', () => { render(); - await userEvent.type(screen.getByLabelText('Username input field'), 'admin'); - await userEvent.type(screen.getByLabelText('Password input field'), 'test'); - await userEvent.click(screen.getByRole('button', { name: 'Login button' })); + await userEvent.type(screen.getByLabelText('Email or username'), 'admin'); + await userEvent.type(screen.getByLabelText('Password'), 'test'); + await userEvent.click(screen.getByRole('button', { name: 'Log in' })); const alert = await screen.findByRole('alert', { name: 'Login failed' }); expect(alert).toBeInTheDocument(); diff --git a/public/app/core/components/Login/LoginPage.tsx b/public/app/core/components/Login/LoginPage.tsx index e86dd4a3b28..cc408a17a43 100644 --- a/public/app/core/components/Login/LoginPage.tsx +++ b/public/app/core/components/Login/LoginPage.tsx @@ -3,7 +3,8 @@ import { css } from '@emotion/css'; import React from 'react'; // Components -import { Alert, HorizontalGroup, LinkButton } from '@grafana/ui'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Alert, HorizontalGroup, LinkButton, useStyles2 } from '@grafana/ui'; import { Branding } from 'app/core/components/Branding/Branding'; import config from 'app/core/config'; import { t } from 'app/core/internationalization'; @@ -16,17 +17,10 @@ import { LoginLayout, InnerBox } from './LoginLayout'; import { LoginServiceButtons } from './LoginServiceButtons'; import { UserSignup } from './UserSignup'; -const forgottenPasswordStyles = css` - padding: 0; - margin-top: 4px; -`; - -const alertStyles = css({ - width: '100%', -}); - export const LoginPage = () => { + const styles = useStyles2(getStyles); document.title = Branding.AppTitle; + return ( {({ @@ -46,7 +40,7 @@ export const LoginPage = () => { {!isChangingPassword && ( {loginErrorMessage && ( - + {loginErrorMessage} )} @@ -55,7 +49,7 @@ export const LoginPage = () => { @@ -83,3 +77,16 @@ export const LoginPage = () => { ); }; + +const getStyles = (theme: GrafanaTheme2) => { + return { + forgottenPassword: css({ + padding: 0, + marginTop: theme.spacing(0.5), + }), + + alert: css({ + width: '100%', + }), + }; +}; diff --git a/public/app/core/components/Login/LoginServiceButtons.tsx b/public/app/core/components/Login/LoginServiceButtons.tsx index aee1e887ba0..eefd6e61669 100644 --- a/public/app/core/components/Login/LoginServiceButtons.tsx +++ b/public/app/core/components/Login/LoginServiceButtons.tsx @@ -77,30 +77,30 @@ const loginServices: () => LoginServices = () => { const getServiceStyles = (theme: GrafanaTheme2) => { return { - button: css` - color: #d8d9da; - position: relative; - `, - buttonIcon: css` - position: absolute; - left: ${theme.spacing(1)}; - top: 50%; - transform: translateY(-50%); - `, + button: css({ + color: '#d8d9da', + position: 'relative', + }), + buttonIcon: css({ + position: 'absolute', + left: theme.spacing(1), + top: '50%', + transform: 'translateY(-50%)', + }), divider: { - base: css` - color: ${theme.colors.text}; - display: flex; - margin-bottom: ${theme.spacing(1)}; - justify-content: space-between; - text-align: center; - width: 100%; - `, - line: css` - width: 100px; - height: 10px; - border-bottom: 1px solid ${theme.colors.text}; - `, + base: css({ + color: theme.colors.text.primary, + display: 'flex', + marginBottom: theme.spacing(1), + justifyContent: 'space-between', + textAlign: 'center', + width: '100%', + }), + line: css({ + width: 100, + height: 10, + borderBottom: `1px solid ${theme.colors.text}`, + }), }, }; }; @@ -128,15 +128,15 @@ const LoginDivider = () => { function getButtonStyleFor(service: LoginService, styles: ReturnType, theme: GrafanaTheme2) { return cx( styles.button, - css` - background-color: ${service.bgColor}; - color: ${theme.colors.getContrastText(service.bgColor)}; + css({ + backgroundColor: service.bgColor, + color: theme.colors.getContrastText(service.bgColor), - &:hover { - background-color: ${theme.colors.emphasize(service.bgColor, 0.15)}; - box-shadow: ${theme.shadows.z1}; - } - ` + ['&:hover']: { + backgroundColor: theme.colors.emphasize(service.bgColor, 0.15), + boxShadow: theme.shadows.z1, + }, + }) ); } diff --git a/public/app/core/components/Login/UserSignup.tsx b/public/app/core/components/Login/UserSignup.tsx index 04866de95d9..56559860549 100644 --- a/public/app/core/components/Login/UserSignup.tsx +++ b/public/app/core/components/Login/UserSignup.tsx @@ -12,10 +12,10 @@ export const UserSignup = () => {
New to Grafana?
{ - const props = { - id: 'password', - placeholder: 'enter password', - 'data-testid': 'password-field', - }; it('should render correctly', () => { - render(); - expect(screen.getByTestId('password-field')).toBeInTheDocument(); + render( + + + + ); + + expect(screen.getByLabelText('Password')).toBeInTheDocument(); expect(screen.getByRole('switch', { name: 'Show password' })).toBeInTheDocument(); }); + it('should able to show password value if clicked on password-reveal icon', () => { - render(); - expect(screen.getByTestId('password-field')).toHaveProperty('type', 'password'); + render( + + + + ); + + expect(screen.getByLabelText('Password')).toHaveProperty('type', 'password'); fireEvent.click(screen.getByRole('switch', { name: 'Show password' })); - expect(screen.getByTestId('password-field')).toHaveProperty('type', 'text'); + expect(screen.getByLabelText('Password')).toHaveProperty('type', 'text'); }); }); diff --git a/public/app/core/components/PasswordField/PasswordField.tsx b/public/app/core/components/PasswordField/PasswordField.tsx index 06a54869722..0a91c8a9b50 100644 --- a/public/app/core/components/PasswordField/PasswordField.tsx +++ b/public/app/core/components/PasswordField/PasswordField.tsx @@ -2,43 +2,33 @@ import React, { useState } from 'react'; import { selectors } from '@grafana/e2e-selectors'; import { Input, IconButton } from '@grafana/ui'; +import { Props as InputProps } from '@grafana/ui/src/components/Input/Input'; -export interface Props { - autoFocus?: boolean; - autoComplete?: string; - id?: string; - passwordHint?: string; -} +interface Props extends Omit {} -export const PasswordField = React.forwardRef( - ({ autoComplete, autoFocus, id, passwordHint, ...props }, ref) => { - const [showPassword, setShowPassword] = useState(false); +export const PasswordField = React.forwardRef((props, ref) => { + const [showPassword, setShowPassword] = useState(false); - return ( - { - setShowPassword(!showPassword); - }} - tooltip={showPassword ? 'Hide password' : 'Show password'} - /> - } - /> - ); - } -); + return ( + { + setShowPassword(!showPassword); + }} + tooltip={showPassword ? 'Hide password' : 'Show password'} + /> + } + /> + ); +}); PasswordField.displayName = 'PasswordField';