From 7728bf2cd8b0739be692599b3484a8afbff2fcbe Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 7 Oct 2024 15:04:39 +0300 Subject: [PATCH] fix: [Obs Services > Create Service Group modal][KEYBOARD]: Focus should not be auto-set on first input when modal appears (#194696) Closes: https://github.com/elastic/kibana/issues/194965 Closes: https://github.com/elastic/kibana/issues/194966 # Description - [x] https://github.com/elastic/kibana/issues/194965
Focus is currently being set on the first input in the "Create group" modal. Screen reader users will hear the input name, but not get the title of the modal read aloud this way, and it could be confusing. We should be letting the EuiModal set focus naturally on the modal or close button so screen reader users hear the title as expected. - [x] https://github.com/elastic/kibana/issues/194966
Focus must be returned properly when I cancel the "Create group" workflow in Services > Create service group modal. # Changes Made 1. Removed: ```diff - inputRef.current?.focus(); // autofocus on initial render ``` 2. Added `aria-labelledby={modalTitleId}` for `EuiModal`. See https://eui.elastic.co/#/layout/modal. 3. Slightly updated `Name` and `Color` validation. # Screen https://github.com/user-attachments/assets/6636f2dc-b9b7-4d4d-8144-90249f8327e7 (cherry picked from commit d5763658c39856aefb5e15fa9e3e771f8bb0d613) --- .../service_group_save/group_details.tsx | 57 ++++++++++++------- .../service_group_save/save_modal.tsx | 8 ++- .../service_group_save/select_services.tsx | 4 +- 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/group_details.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/group_details.tsx index dccfc2be9da27..5e82ab290e07d 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/group_details.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/group_details.tsx @@ -21,7 +21,7 @@ import { isValidHex, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useState } from 'react'; import type { StagedServiceGroup } from './save_modal'; interface Props { @@ -31,6 +31,7 @@ interface Props { onClickNext: (serviceGroup: StagedServiceGroup) => void; onDeleteGroup: () => void; isLoading: boolean; + titleId?: string; } export function GroupDetails({ @@ -40,13 +41,16 @@ export function GroupDetails({ onClickNext, onDeleteGroup, isLoading, + titleId, }: Props) { - const [name, setName] = useState(serviceGroup?.groupName || ''); - const [color, setColor, colorPickerErrors] = useColorPickerState( - serviceGroup?.color || '#5094C4' - ); - + const initialColor = serviceGroup?.color || '#5094C4'; + const [name, setName] = useState(serviceGroup?.groupName); + const [color, setColor, colorPickerErrors] = useColorPickerState(initialColor); const [description, setDescription] = useState(serviceGroup?.description); + + const isNamePristine = name === serviceGroup?.groupName; + const isColorPristine = color === initialColor; + useEffect(() => { if (serviceGroup) { setName(serviceGroup.groupName); @@ -65,16 +69,10 @@ export function GroupDetails({ const isInvalidName = !name; const isInvalid = isInvalidName || isInvalidColor; - const inputRef = useRef(null); - - useEffect(() => { - inputRef.current?.focus(); // autofocus on initial render - }, []); - return ( <> - + {isEdit ? i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.edit.title', { defaultMessage: 'Edit group', @@ -93,15 +91,25 @@ export function GroupDetails({ label={i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.name', { defaultMessage: 'Name', })} - isInvalid={isInvalidName} + isInvalid={!isNamePristine && isInvalidName} + error={ + !isNamePristine && isInvalidName + ? i18n.translate( + 'xpack.apm.serviceGroups.groupDetailsForm.invalidNameError', + { + defaultMessage: 'Please provide a valid name value', + } + ) + : undefined + } > { setName(e.target.value); }} - inputRef={inputRef} + isInvalid={!isNamePristine && isInvalidName} /> @@ -110,9 +118,9 @@ export function GroupDetails({ label={i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.color', { defaultMessage: 'Color', })} - isInvalid={isInvalidColor} + isInvalid={!isColorPristine && isInvalidColor} error={ - isInvalidColor + !isColorPristine && isInvalidColor ? i18n.translate( 'xpack.apm.serviceGroups.groupDetailsForm.invalidColorError', { @@ -122,7 +130,11 @@ export function GroupDetails({ : undefined } > - + @@ -144,7 +156,7 @@ export function GroupDetails({ { setDescription(e.target.value); }} @@ -164,6 +176,7 @@ export function GroupDetails({ onDeleteGroup(); }} color="danger" + isLoading={isLoading} isDisabled={isLoading} data-test-subj="apmDeleteGroupButton" > @@ -177,6 +190,7 @@ export function GroupDetails({ {i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.cancel', { @@ -192,12 +206,13 @@ export function GroupDetails({ iconSide="right" onClick={() => { onClickNext({ - groupName: name, + groupName: name || '', color, description, kuery: serviceGroup?.kuery ?? '', }); }} + isLoading={isLoading} isDisabled={isInvalid || isLoading} > {i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.selectServices', { diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/save_modal.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/save_modal.tsx index ee16952fddb79..9ecc00d8681ec 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/save_modal.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/save_modal.tsx @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { EuiModal } from '@elastic/eui'; +import { EuiModal, useGeneratedHtmlId } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { useHistory } from 'react-router-dom'; import React, { useCallback, useEffect, useState } from 'react'; @@ -89,6 +89,8 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) { [savedServiceGroup?.id, notifications.toasts, onClose, isEdit, navigateToServiceGroups] ); + const modalTitleId = useGeneratedHtmlId(); + const onDelete = useCallback( async function () { setIsLoading(true); @@ -115,7 +117,7 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) { ); return ( - + {modalView === 'group_details' && ( )} {modalView === 'select_service' && stagedServiceGroup && ( @@ -139,6 +142,7 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) { setModalView('group_details'); }} isLoading={isLoading} + titleId={modalTitleId} /> )} diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/select_services.tsx index 5b7ce6607ca05..b6a901bac8d2f 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/select_services.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/select_services.tsx @@ -54,6 +54,7 @@ interface Props { onSaveClick: (serviceGroup: StagedServiceGroup) => void; onEditGroupDetailsClick: () => void; isLoading: boolean; + titleId?: string; } export function SelectServices({ @@ -63,6 +64,7 @@ export function SelectServices({ onSaveClick, onEditGroupDetailsClick, isLoading, + titleId, }: Props) { const [kuery, setKuery] = useState(serviceGroup?.kuery || ''); const [stagedKuery, setStagedKuery] = useState(serviceGroup?.kuery || ''); @@ -117,7 +119,7 @@ export function SelectServices({
- + {i18n.translate('xpack.apm.serviceGroups.selectServicesForm.title', { defaultMessage: 'Select services', })}