From c8b737301635fe6d17ef487f1e4694a158ed5cd6 Mon Sep 17 00:00:00 2001 From: kay delaney <45561153+kaydelaney@users.noreply.github.com> Date: Fri, 5 Nov 2021 14:45:00 +0000 Subject: [PATCH] A11y/UserAdminPage: Improves tab navigation and focus management (#41321) --- .../ConfirmButton/ConfirmButton.tsx | 45 ++- .../components/ConfirmModal/ConfirmModal.tsx | 6 +- .../Forms/RadioButtonGroup/RadioButton.tsx | 71 +++-- .../RadioButtonGroup/RadioButtonGroup.tsx | 12 +- .../app/core/components/Select/OrgPicker.tsx | 80 ++--- public/app/features/admin/OrgRolePicker.tsx | 35 ++- public/app/features/admin/UserOrgs.tsx | 20 +- public/app/features/admin/UserPermissions.tsx | 143 ++++----- public/app/features/admin/UserProfile.tsx | 274 ++++++++---------- public/app/features/admin/UserSessions.tsx | 16 +- .../features/api-keys/ApiKeysPage.test.tsx | 25 +- .../AnnotationsSettings.test.tsx | 18 +- .../DashboardSettings/LinksSettings.test.tsx | 6 +- 13 files changed, 369 insertions(+), 382 deletions(-) diff --git a/packages/grafana-ui/src/components/ConfirmButton/ConfirmButton.tsx b/packages/grafana-ui/src/components/ConfirmButton/ConfirmButton.tsx index 7877650d01a..d41edf0b699 100644 --- a/packages/grafana-ui/src/components/ConfirmButton/ConfirmButton.tsx +++ b/packages/grafana-ui/src/components/ConfirmButton/ConfirmButton.tsx @@ -21,6 +21,8 @@ export interface Props extends Themeable { confirmVariant?: ButtonVariant; /** Hide confirm actions when after of them is clicked */ closeOnConfirm?: boolean; + /** Move focus to button when mounted */ + autoFocus?: boolean; /** Optional on click handler for the original button */ onClick?(): void; @@ -33,6 +35,8 @@ interface State { } class UnThemedConfirmButton extends PureComponent { + mainButtonRef = React.createRef(); + confirmButtonRef = React.createRef(); state: State = { showConfirm: false, }; @@ -42,9 +46,16 @@ class UnThemedConfirmButton extends PureComponent { event.preventDefault(); } - this.setState({ - showConfirm: true, - }); + this.setState( + { + showConfirm: true, + }, + () => { + if (this.props.autoFocus && this.confirmButtonRef.current) { + this.confirmButtonRef.current.focus(); + } + } + ); if (this.props.onClick) { this.props.onClick(); @@ -55,9 +66,14 @@ class UnThemedConfirmButton extends PureComponent { if (event) { event.preventDefault(); } - this.setState({ - showConfirm: false, - }); + this.setState( + { + showConfirm: false, + }, + () => { + this.mainButtonRef.current?.focus(); + } + ); if (this.props.onCancel) { this.props.onCancel(); } @@ -101,7 +117,7 @@ class UnThemedConfirmButton extends PureComponent { {typeof children === 'string' ? ( - @@ -111,12 +127,12 @@ class UnThemedConfirmButton extends PureComponent { )} + - ); @@ -128,9 +144,9 @@ export const ConfirmButton = withTheme(UnThemedConfirmButton); const getStyles = stylesFactory((theme: GrafanaTheme) => { return { buttonContainer: css` - direction: rtl; display: flex; align-items: center; + justify-content: flex-end; `, buttonDisabled: css` text-decoration: none; @@ -146,14 +162,14 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { `, buttonHide: css` opacity: 0; - transition: opacity 0.1s ease; + transition: opacity 0.1s ease, visibility 0 0.1s; + visibility: hidden; z-index: 0; `, confirmButton: css` align-items: flex-start; background: ${theme.colors.bg1}; display: flex; - overflow: hidden; position: absolute; pointer-events: none; `, @@ -166,7 +182,8 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { `, confirmButtonHide: css` opacity: 0; - transition: opacity 0.12s ease-in, transform 0.14s ease-in; + visibility: hidden; + transition: opacity 0.12s ease-in, transform 0.14s ease-in, visibility 0s 0.12s; transform: translateX(100px); `, }; diff --git a/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx b/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx index ee8071c6f00..d2c64cd404e 100644 --- a/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx +++ b/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx @@ -58,8 +58,10 @@ export const ConfirmModal = ({ useEffect(() => { // for some reason autoFocus property did no work on this button, but this does - buttonRef.current?.focus(); - }, []); + if (isOpen) { + buttonRef.current?.focus(); + } + }, [isOpen]); return ( diff --git a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx index 257255ebdb6..2ea6a1a8525 100644 --- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx +++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx @@ -18,41 +18,48 @@ export interface RadioButtonProps { onChange: () => void; fullWidth?: boolean; 'aria-label'?: StringSelector; + children?: React.ReactNode; } -export const RadioButton: React.FC = ({ - children, - active = false, - disabled = false, - size = 'md', - onChange, - id, - name = undefined, - description, - fullWidth, - 'aria-label': ariaLabel, -}) => { - const theme = useTheme2(); - const styles = getRadioButtonStyles(theme, size, fullWidth); +export const RadioButton = React.forwardRef( + ( + { + children, + active = false, + disabled = false, + size = 'md', + onChange, + id, + name = undefined, + description, + fullWidth, + 'aria-label': ariaLabel, + }, + ref + ) => { + const theme = useTheme2(); + const styles = getRadioButtonStyles(theme, size, fullWidth); - return ( - <> - - - - ); -}; + return ( + <> + + + + ); + } +); RadioButton.displayName = 'RadioButton'; diff --git a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx index c8bc83deff4..d54371fe95a 100644 --- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx +++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef } from 'react'; +import React, { useCallback, useEffect, useRef } from 'react'; import { css, cx } from '@emotion/css'; import { uniqueId } from 'lodash'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; @@ -16,6 +16,7 @@ export interface RadioButtonGroupProps { size?: RadioButtonSize; fullWidth?: boolean; className?: string; + autoFocus?: boolean; } export function RadioButtonGroup({ @@ -27,6 +28,7 @@ export function RadioButtonGroup({ size = 'md', className, fullWidth = false, + autoFocus = false, }: RadioButtonGroupProps) { const handleOnChange = useCallback( (option: SelectableValue) => { @@ -42,6 +44,13 @@ export function RadioButtonGroup({ const groupName = useRef(id); const styles = useStyles2(getStyles); + const activeButtonRef = useRef(null); + useEffect(() => { + if (autoFocus && activeButtonRef.current) { + activeButtonRef.current.focus(); + } + }, [autoFocus]); + return (
{options.map((o, i) => { @@ -58,6 +67,7 @@ export function RadioButtonGroup({ name={groupName.current} description={o.description} fullWidth={fullWidth} + ref={value === o.value ? activeButtonRef : undefined} > {o.icon && } {o.imgUrl && {o.label}} diff --git a/public/app/core/components/Select/OrgPicker.tsx b/public/app/core/components/Select/OrgPicker.tsx index 9690b9a5c73..b30d24015f3 100644 --- a/public/app/core/components/Select/OrgPicker.tsx +++ b/public/app/core/components/Select/OrgPicker.tsx @@ -1,8 +1,9 @@ -import React, { PureComponent } from 'react'; +import React, { useEffect } from 'react'; import { AsyncSelect } from '@grafana/ui'; import { getBackendSrv } from 'app/core/services/backend_srv'; import { Organization } from 'app/types'; import { SelectableValue } from '@grafana/data'; +import { useAsyncFn } from 'react-use'; export type OrgSelectItem = SelectableValue; @@ -10,56 +11,35 @@ export interface Props { onSelected: (org: OrgSelectItem) => void; className?: string; inputId?: string; + autoFocus?: boolean; } -export interface State { - isLoading: boolean; -} - -export class OrgPicker extends PureComponent { - orgs: Organization[] = []; - - state: State = { - isLoading: false, - }; - - async loadOrgs() { - this.setState({ isLoading: true }); - const orgs = await getBackendSrv().get('/api/orgs'); - this.orgs = orgs; - this.setState({ isLoading: false }); - return orgs; - } - - getOrgOptions = async (query: string): Promise => { - if (!this.orgs?.length) { - await this.loadOrgs(); +export function OrgPicker({ onSelected, className, inputId, autoFocus }: Props) { + // For whatever reason the autoFocus prop doesn't seem to work + // with AsyncSelect, hence this workaround. Maybe fixed in a later version? + useEffect(() => { + if (autoFocus && inputId) { + document.getElementById(inputId)?.focus(); } - return this.orgs.map( - (org: Organization): OrgSelectItem => ({ - value: { id: org.id, name: org.name }, - label: org.name, - }) - ); - }; - - render() { - const { className, onSelected, inputId } = this.props; - const { isLoading } = this.state; - - return ( - - ); - } + }, [autoFocus, inputId]); + + const [orgOptionsState, getOrgOptions] = useAsyncFn(async () => { + const orgs: Organization[] = await getBackendSrv().get('/api/orgs'); + return orgs.map((org) => ({ value: { id: org.id, name: org.name }, label: org.name })); + }); + + return ( + + ); } diff --git a/public/app/features/admin/OrgRolePicker.tsx b/public/app/features/admin/OrgRolePicker.tsx index bae10a5d5b9..0335f827ce6 100644 --- a/public/app/features/admin/OrgRolePicker.tsx +++ b/public/app/features/admin/OrgRolePicker.tsx @@ -8,19 +8,30 @@ interface Props { 'aria-label'?: string; inputId?: string; onChange: (role: OrgRole) => void; + autoFocus?: boolean; } const options = Object.keys(OrgRole).map((key) => ({ label: key, value: key })); -export const OrgRolePicker: FC = ({ value, onChange, 'aria-label': ariaLabel, inputId, ...restProps }) => ( - onChange(val.value as OrgRole)} + placeholder="Choose role..." + aria-label={ariaLabel} + autoFocus={autoFocus} + {...restProps} + /> + ); +}; diff --git a/public/app/features/admin/UserOrgs.tsx b/public/app/features/admin/UserOrgs.tsx index 558682f03f8..588c748d4d1 100644 --- a/public/app/features/admin/UserOrgs.tsx +++ b/public/app/features/admin/UserOrgs.tsx @@ -33,12 +33,19 @@ interface State { } export class UserOrgs extends PureComponent { + addToOrgButtonRef = React.createRef(); state = { showAddOrgModal: false, }; - showOrgAddModal = (show: boolean) => () => { - this.setState({ showAddOrgModal: show }); + showOrgAddModal = () => { + this.setState({ showAddOrgModal: true }); + }; + + dismissOrgAddModal = () => { + this.setState({ showAddOrgModal: false }, () => { + this.addToOrgButtonRef.current?.focus(); + }); }; render() { @@ -69,12 +76,12 @@ export class UserOrgs extends PureComponent {
{canAddToOrg && ( - )}
- + ); @@ -159,7 +166,7 @@ class UnThemedOrgRow extends PureComponent { {isChangingRole ? ( - + ) : ( {org.role} @@ -184,6 +191,7 @@ class UnThemedOrgRow extends PureComponent { confirmVariant="destructive" onCancel={this.onCancelClick} onConfirm={this.onOrgRemove} + autoFocus > Remove from organization @@ -260,7 +268,7 @@ export class AddToOrgModal extends PureComponent - + diff --git a/public/app/features/admin/UserPermissions.tsx b/public/app/features/admin/UserPermissions.tsx index 235c52870b0..f1888f1a739 100644 --- a/public/app/features/admin/UserPermissions.tsx +++ b/public/app/features/admin/UserPermissions.tsx @@ -1,6 +1,5 @@ -import React, { PureComponent } from 'react'; +import React, { useState } from 'react'; import { ConfirmButton, RadioButtonGroup, Icon } from '@grafana/ui'; -import { cx } from '@emotion/css'; import { AccessControlAction } from 'app/types'; import { contextSrv } from 'app/core/core'; @@ -10,98 +9,72 @@ interface Props { onGrafanaAdminChange: (isGrafanaAdmin: boolean) => void; } -interface State { - isEditing: boolean; - currentAdminOption: string; -} - const adminOptions = [ - { label: 'Yes', value: 'YES' }, - { label: 'No', value: 'NO' }, + { label: 'Yes', value: true }, + { label: 'No', value: false }, ]; -export class UserPermissions extends PureComponent { - state = { - isEditing: false, - currentAdminOption: this.props.isGrafanaAdmin ? 'YES' : 'NO', - }; - - onChangeClick = () => { - this.setState({ isEditing: true }); - }; +export function UserPermissions({ isGrafanaAdmin, onGrafanaAdminChange }: Props) { + const [isEditing, setIsEditing] = useState(false); + const [currentAdminOption, setCurrentAdminOption] = useState(isGrafanaAdmin); - onCancelClick = () => { - this.setState({ - isEditing: false, - currentAdminOption: this.props.isGrafanaAdmin ? 'YES' : 'NO', - }); - }; + const onChangeClick = () => setIsEditing(true); - onGrafanaAdminChange = () => { - const { currentAdminOption } = this.state; - const newIsGrafanaAdmin = currentAdminOption === 'YES' ? true : false; - this.props.onGrafanaAdminChange(newIsGrafanaAdmin); + const onCancelClick = () => { + setIsEditing(false); + setCurrentAdminOption(isGrafanaAdmin); }; - onAdminOptionSelect = (value: string) => { - this.setState({ currentAdminOption: value }); - }; + const handleGrafanaAdminChange = () => onGrafanaAdminChange(currentAdminOption); - render() { - const { isGrafanaAdmin } = this.props; - const { isEditing, currentAdminOption } = this.state; - const changeButtonContainerClass = cx('pull-right'); - const canChangePermissions = contextSrv.hasPermission(AccessControlAction.UsersPermissionsUpdate); + const canChangePermissions = contextSrv.hasPermission(AccessControlAction.UsersPermissionsUpdate); - return ( - <> -

Permissions

-
-
- - - - - {isEditing ? ( - - ) : ( - - )} - + )} + + + +
Grafana Admin - - - {isGrafanaAdmin ? ( - <> - Yes - - ) : ( - <>No - )} - -
- {canChangePermissions && ( - - Change - - )} -
+ return ( + <> +

Permissions

+
+
+ + + + + {isEditing ? ( + - - -
Grafana Admin +
-
+ ) : ( +
+ {isGrafanaAdmin ? ( + <> + Yes + + ) : ( + <>No + )} + + {canChangePermissions && ( + + Change + + )} +
- - ); - } +
+ + ); } diff --git a/public/app/features/admin/UserProfile.tsx b/public/app/features/admin/UserProfile.tsx index 74501ceb012..f036d765356 100644 --- a/public/app/features/admin/UserProfile.tsx +++ b/public/app/features/admin/UserProfile.tsx @@ -1,4 +1,4 @@ -import React, { FC, PureComponent } from 'react'; +import React, { FC, PureComponent, useRef, useState } from 'react'; import { AccessControlAction, UserDTO } from 'app/types'; import { css, cx } from '@emotion/css'; import { config } from 'app/core/config'; @@ -16,163 +16,149 @@ interface Props { onPasswordChange(password: string): void; } -interface State { - isLoading: boolean; - showDeleteModal: boolean; - showDisableModal: boolean; -} - -export class UserProfile extends PureComponent { - state = { - isLoading: false, - showDeleteModal: false, - showDisableModal: false, +export function UserProfile({ + user, + onUserUpdate, + onUserDelete, + onUserDisable, + onUserEnable, + onPasswordChange, +}: Props) { + const [showDeleteModal, setShowDeleteModal] = useState(false); + const [showDisableModal, setShowDisableModal] = useState(false); + + const deleteUserRef = useRef(null); + const showDeleteUserModal = (show: boolean) => () => { + setShowDeleteModal(show); + if (!show && deleteUserRef.current) { + deleteUserRef.current.focus(); + } }; - showDeleteUserModal = (show: boolean) => () => { - this.setState({ showDeleteModal: show }); + const disableUserRef = useRef(null); + const showDisableUserModal = (show: boolean) => () => { + setShowDisableModal(show); + if (!show && disableUserRef.current) { + disableUserRef.current.focus(); + } }; - showDisableUserModal = (show: boolean) => () => { - this.setState({ showDisableModal: show }); - }; + const handleUserDelete = () => onUserDelete(user.id); - onUserDelete = () => { - const { user, onUserDelete } = this.props; - onUserDelete(user.id); - }; + const handleUserDisable = () => onUserDisable(user.id); - onUserDisable = () => { - const { user, onUserDisable } = this.props; - onUserDisable(user.id); - }; - - onUserEnable = () => { - const { user, onUserEnable } = this.props; - onUserEnable(user.id); - }; + const handleUserEnable = () => onUserEnable(user.id); - onUserNameChange = (newValue: string) => { - const { user, onUserUpdate } = this.props; + const onUserNameChange = (newValue: string) => { onUserUpdate({ ...user, name: newValue, }); }; - onUserEmailChange = (newValue: string) => { - const { user, onUserUpdate } = this.props; + const onUserEmailChange = (newValue: string) => { onUserUpdate({ ...user, email: newValue, }); }; - onUserLoginChange = (newValue: string) => { - const { user, onUserUpdate } = this.props; + const onUserLoginChange = (newValue: string) => { onUserUpdate({ ...user, login: newValue, }); }; - onPasswordChange = (newValue: string) => { - this.props.onPasswordChange(newValue); - }; + const authSource = user.authLabels?.length && user.authLabels[0]; + const lockMessage = authSource ? `Synced via ${authSource}` : ''; + const styles = getStyles(config.theme); - render() { - const { user } = this.props; - const { showDeleteModal, showDisableModal } = this.state; - const authSource = user.authLabels?.length && user.authLabels[0]; - const lockMessage = authSource ? `Synced via ${authSource}` : ''; - const styles = getStyles(config.theme); - - const editLocked = user.isExternal || !contextSrv.hasPermission(AccessControlAction.UsersWrite); - const passwordChangeLocked = user.isExternal || !contextSrv.hasPermission(AccessControlAction.UsersPasswordUpdate); - const canDelete = contextSrv.hasPermission(AccessControlAction.UsersDelete); - const canDisable = contextSrv.hasPermission(AccessControlAction.UsersDisable); - const canEnable = contextSrv.hasPermission(AccessControlAction.UsersEnable); + const editLocked = user.isExternal || !contextSrv.hasPermission(AccessControlAction.UsersWrite); + const passwordChangeLocked = user.isExternal || !contextSrv.hasPermission(AccessControlAction.UsersPasswordUpdate); + const canDelete = contextSrv.hasPermission(AccessControlAction.UsersDelete); + const canDisable = contextSrv.hasPermission(AccessControlAction.UsersDisable); + const canEnable = contextSrv.hasPermission(AccessControlAction.UsersEnable); - return ( - <> -

User information

-
-
- - - - - - - -
-
-
- {canDelete && ( - <> - - - - )} - {user.isDisabled && canEnable && ( - + + + )} + {user.isDisabled && canEnable && ( + + )} + {!user.isDisabled && canDisable && ( + <> + - )} - {!user.isDisabled && canDisable && ( - <> - - - - )} -
+ + + )}
- - ); - } + + + ); } const getStyles = stylesFactory((theme: GrafanaTheme) => { @@ -269,7 +255,6 @@ export class UserProfileRow extends PureComponent; @@ -297,16 +282,14 @@ export class UserProfileRow extends PureComponent -
- - Edit - -
+ + Edit + ); @@ -320,13 +303,10 @@ interface LockedRowProps { } export const LockedRow: FC = ({ label, value, lockMessage }) => { - const lockMessageClass = cx( - 'pull-right', - css` - font-style: italic; - margin-right: 0.6rem; - ` - ); + const lockMessageClass = css` + font-style: italic; + margin-right: 0.6rem; + `; const labelClass = cx( 'width-16', css` diff --git a/public/app/features/admin/UserSessions.tsx b/public/app/features/admin/UserSessions.tsx index 1ff8431eea4..0b9d530d981 100644 --- a/public/app/features/admin/UserSessions.tsx +++ b/public/app/features/admin/UserSessions.tsx @@ -16,12 +16,19 @@ interface State { } export class UserSessions extends PureComponent { + forceAllLogoutButton = React.createRef(); state: State = { showLogoutModal: false, }; - showLogoutConfirmationModal = (show: boolean) => () => { - this.setState({ showLogoutModal: show }); + showLogoutConfirmationModal = () => { + this.setState({ showLogoutModal: true }); + }; + + dismissLogoutConfirmationModal = () => { + this.setState({ showLogoutModal: false }, () => { + this.forceAllLogoutButton.current?.focus(); + }); }; onSessionRevoke = (id: number) => { @@ -74,6 +81,7 @@ export class UserSessions extends PureComponent { confirmText="Confirm logout" confirmVariant="destructive" onConfirm={this.onSessionRevoke(session.id)} + autoFocus > Force logout @@ -87,7 +95,7 @@ export class UserSessions extends PureComponent {
{canLogout && sessions.length > 0 && ( - )} @@ -97,7 +105,7 @@ export class UserSessions extends PureComponent { body="Are you sure you want to force logout from all devices?" confirmText="Force logout" onConfirm={this.onAllSessionsRevoke} - onDismiss={this.showLogoutConfirmationModal(false)} + onDismiss={this.dismissLogoutConfirmationModal} />
diff --git a/public/app/features/api-keys/ApiKeysPage.test.tsx b/public/app/features/api-keys/ApiKeysPage.test.tsx index b28b60ca606..46d22b22e8a 100644 --- a/public/app/features/api-keys/ApiKeysPage.test.tsx +++ b/public/app/features/api-keys/ApiKeysPage.test.tsx @@ -75,9 +75,9 @@ describe('ApiKeysPage', () => { setup({ apiKeys, apiKeysCount: apiKeys.length, hasFetched: true }); expect(screen.getByRole('table')).toBeInTheDocument(); expect(screen.getAllByRole('row').length).toBe(4); - expect(screen.getByRole('row', { name: /first admin 2021-01-01 00:00:00 cancel delete/i })).toBeInTheDocument(); - expect(screen.getByRole('row', { name: /second editor 2021-01-02 00:00:00 cancel delete/i })).toBeInTheDocument(); - expect(screen.getByRole('row', { name: /third viewer no expiration date cancel delete/i })).toBeInTheDocument(); + expect(screen.getByRole('row', { name: /first admin 2021-01-01 00:00:00/i })).toBeInTheDocument(); + expect(screen.getByRole('row', { name: /second editor 2021-01-02 00:00:00/i })).toBeInTheDocument(); + expect(screen.getByRole('row', { name: /third viewer no expiration date/i })).toBeInTheDocument(); }); }); @@ -118,27 +118,24 @@ describe('ApiKeysPage', () => { { id: 3, name: 'Third', role: OrgRole.Viewer, secondsToLive: 0, expiration: undefined }, ]; const { deleteApiKeyMock } = setup({ apiKeys, apiKeysCount: apiKeys.length, hasFetched: true }); - const firstRow = screen.getByRole('row', { name: /first admin 2021-01-01 00:00:00 cancel delete/i }); - const secondRow = screen.getByRole('row', { name: /second editor 2021-01-02 00:00:00 cancel delete/i }); + const firstRow = screen.getByRole('row', { name: /first admin 2021-01-01 00:00:00/i }); + const secondRow = screen.getByRole('row', { name: /second editor 2021-01-02 00:00:00/i }); deleteApiKeyMock.mockClear(); - expect(within(firstRow).getByRole('cell', { name: /cancel delete/i })).toBeInTheDocument(); - userEvent.click(within(firstRow).getByRole('cell', { name: /cancel delete/i })); + expect(within(firstRow).getByLabelText('Delete API key')).toBeInTheDocument(); + userEvent.click(within(firstRow).getByLabelText('Delete API key')); + expect(within(firstRow).getByRole('button', { name: /delete$/i })).toBeInTheDocument(); - // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed - userEvent.click(within(firstRow).getByRole('button', { name: /delete$/i }), undefined, { - skipPointerEventsCheck: true, - }); + userEvent.click(within(firstRow).getByRole('button', { name: /delete$/i })); expect(deleteApiKeyMock).toHaveBeenCalledTimes(1); expect(deleteApiKeyMock).toHaveBeenCalledWith(1, false); toggleShowExpired(); deleteApiKeyMock.mockClear(); - expect(within(secondRow).getByRole('cell', { name: /cancel delete/i })).toBeInTheDocument(); - userEvent.click(within(secondRow).getByRole('cell', { name: /cancel delete/i })); + expect(within(secondRow).getByLabelText('Delete API key')).toBeInTheDocument(); + userEvent.click(within(secondRow).getByLabelText('Delete API key')); expect(within(secondRow).getByRole('button', { name: /delete$/i })).toBeInTheDocument(); - // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed userEvent.click(within(secondRow).getByRole('button', { name: /delete$/i }), undefined, { skipPointerEventsCheck: true, }); diff --git a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx index 4a738cf40b2..f81055f8189 100644 --- a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx @@ -116,9 +116,7 @@ describe('AnnotationsSettings', () => { expect(screen.getByRole('heading', { name: /annotations/i })).toBeInTheDocument(); expect(screen.queryByRole('table')).toBeInTheDocument(); - expect( - screen.getByRole('row', { name: /annotations & alerts \(built\-in\) grafana cancel delete/i }) - ).toBeInTheDocument(); + expect(screen.getByRole('row', { name: /annotations & alerts \(built\-in\) grafana/i })).toBeInTheDocument(); expect( screen.queryByLabelText(selectors.components.CallToActionCard.button('Add annotation query')) ).toBeInTheDocument(); @@ -142,14 +140,14 @@ describe('AnnotationsSettings', () => { userEvent.click(within(heading).getByText(/annotations/i)); expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getByRole('row', { name: /my annotation \(built\-in\) grafana cancel delete/i })).toBeInTheDocument(); + expect(screen.getByRole('row', { name: /my annotation \(built\-in\) grafana/i })).toBeInTheDocument(); expect( screen.queryByLabelText(selectors.components.CallToActionCard.button('Add annotation query')) ).toBeInTheDocument(); expect(screen.queryByRole('button', { name: /new query/i })).not.toBeInTheDocument(); - // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed - userEvent.click(screen.getByRole('button', { name: /^delete$/i }), undefined, { skipPointerEventsCheck: true }); + userEvent.click(screen.getAllByLabelText(/Delete query with title/)[0]); + userEvent.click(screen.getByRole('button', { name: 'Delete' })); expect(screen.queryAllByRole('row').length).toBe(0); expect( @@ -238,9 +236,7 @@ describe('AnnotationsSettings', () => { userEvent.click(within(heading).getByText(/annotations/i)); expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(2); - expect( - screen.queryByRole('row', { name: /my prometheus annotation prometheus cancel delete/i }) - ).toBeInTheDocument(); + expect(screen.queryByRole('row', { name: /my prometheus annotation prometheus/i })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: /new query/i })).toBeInTheDocument(); expect( screen.queryByLabelText(selectors.components.CallToActionCard.button('Add annotation query')) @@ -252,8 +248,8 @@ describe('AnnotationsSettings', () => { expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(3); - // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed - userEvent.click(screen.getAllByRole('button', { name: /delete/i })[1], undefined, { skipPointerEventsCheck: true }); + userEvent.click(screen.getAllByLabelText(/Delete query with title/)[0]); + userEvent.click(screen.getByRole('button', { name: 'Delete' })); expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(2); }); diff --git a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx index 3ce382c8ce9..6b2fc419bae 100644 --- a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx @@ -131,10 +131,8 @@ describe('LinksSettings', () => { expect(getTableBodyRows().length).toBe(links.length); - // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed - userEvent.click(within(getTableBody()).getAllByRole('button', { name: 'Delete' })[0], undefined, { - skipPointerEventsCheck: true, - }); + userEvent.click(within(getTableBody()).getAllByLabelText(/Delete link with title/)[0]); + userEvent.click(within(getTableBody()).getByRole('button', { name: 'Delete' })); expect(getTableBodyRows().length).toBe(links.length - 1); expect(within(getTableBody()).queryByText(links[0].title)).not.toBeInTheDocument();