Skip to content

Commit 7728bf2

Browse files
alexwizpalexwizp
committed
fix: [Obs Services > Create Service Group modal][KEYBOARD]: Focus should not be auto-set on first input when modal appears (#194696)
Closes: #194965 Closes: #194966 # Description - [x] #194965 <br /> 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] #194966 <br /> 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 d576365)
1 parent 0776378 commit 7728bf2

3 files changed

Lines changed: 45 additions & 24 deletions

File tree

‎x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/group_details.tsx‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
isValidHex,
2222
} from '@elastic/eui';
2323
import { i18n } from '@kbn/i18n';
24-
import React, { useEffect, useRef, useState } from 'react';
24+
import React, { useEffect, useState } from 'react';
2525
import type { StagedServiceGroup } from './save_modal';
2626

2727
interface Props {
@@ -31,6 +31,7 @@ interface Props {
3131
onClickNext: (serviceGroup: StagedServiceGroup) => void;
3232
onDeleteGroup: () => void;
3333
isLoading: boolean;
34+
titleId?: string;
3435
}
3536

3637
export function GroupDetails({
@@ -40,13 +41,16 @@ export function GroupDetails({
4041
onClickNext,
4142
onDeleteGroup,
4243
isLoading,
44+
titleId,
4345
}: Props) {
44-
const [name, setName] = useState<string>(serviceGroup?.groupName || '');
45-
const [color, setColor, colorPickerErrors] = useColorPickerState(
46-
serviceGroup?.color || '#5094C4'
47-
);
48-
46+
const initialColor = serviceGroup?.color || '#5094C4';
47+
const [name, setName] = useState(serviceGroup?.groupName);
48+
const [color, setColor, colorPickerErrors] = useColorPickerState(initialColor);
4949
const [description, setDescription] = useState<string | undefined>(serviceGroup?.description);
50+
51+
const isNamePristine = name === serviceGroup?.groupName;
52+
const isColorPristine = color === initialColor;
53+
5054
useEffect(() => {
5155
if (serviceGroup) {
5256
setName(serviceGroup.groupName);
@@ -65,16 +69,10 @@ export function GroupDetails({
6569
const isInvalidName = !name;
6670
const isInvalid = isInvalidName || isInvalidColor;
6771

68-
const inputRef = useRef<HTMLInputElement | null>(null);
69-
70-
useEffect(() => {
71-
inputRef.current?.focus(); // autofocus on initial render
72-
}, []);
73-
7472
return (
7573
<>
7674
<EuiModalHeader>
77-
<EuiModalHeaderTitle>
75+
<EuiModalHeaderTitle id={titleId}>
7876
{isEdit
7977
? i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.edit.title', {
8078
defaultMessage: 'Edit group',
@@ -93,15 +91,25 @@ export function GroupDetails({
9391
label={i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.name', {
9492
defaultMessage: 'Name',
9593
})}
96-
isInvalid={isInvalidName}
94+
isInvalid={!isNamePristine && isInvalidName}
95+
error={
96+
!isNamePristine && isInvalidName
97+
? i18n.translate(
98+
'xpack.apm.serviceGroups.groupDetailsForm.invalidNameError',
99+
{
100+
defaultMessage: 'Please provide a valid name value',
101+
}
102+
)
103+
: undefined
104+
}
97105
>
98106
<EuiFieldText
99107
data-test-subj="apmGroupNameInput"
100-
value={name}
108+
value={name || ''}
101109
onChange={(e) => {
102110
setName(e.target.value);
103111
}}
104-
inputRef={inputRef}
112+
isInvalid={!isNamePristine && isInvalidName}
105113
/>
106114
</EuiFormRow>
107115
</EuiFlexItem>
@@ -110,9 +118,9 @@ export function GroupDetails({
110118
label={i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.color', {
111119
defaultMessage: 'Color',
112120
})}
113-
isInvalid={isInvalidColor}
121+
isInvalid={!isColorPristine && isInvalidColor}
114122
error={
115-
isInvalidColor
123+
!isColorPristine && isInvalidColor
116124
? i18n.translate(
117125
'xpack.apm.serviceGroups.groupDetailsForm.invalidColorError',
118126
{
@@ -122,7 +130,11 @@ export function GroupDetails({
122130
: undefined
123131
}
124132
>
125-
<EuiColorPicker onChange={setColor} color={color} isInvalid={isInvalidColor} />
133+
<EuiColorPicker
134+
onChange={setColor}
135+
color={color}
136+
isInvalid={!isColorPristine && isInvalidColor}
137+
/>
126138
</EuiFormRow>
127139
</EuiFlexItem>
128140
</EuiFlexGroup>
@@ -144,7 +156,7 @@ export function GroupDetails({
144156
<EuiFieldText
145157
data-test-subj="apmGroupDetailsFieldText"
146158
fullWidth
147-
value={description}
159+
value={description || ''}
148160
onChange={(e) => {
149161
setDescription(e.target.value);
150162
}}
@@ -164,6 +176,7 @@ export function GroupDetails({
164176
onDeleteGroup();
165177
}}
166178
color="danger"
179+
isLoading={isLoading}
167180
isDisabled={isLoading}
168181
data-test-subj="apmDeleteGroupButton"
169182
>
@@ -177,6 +190,7 @@ export function GroupDetails({
177190
<EuiButtonEmpty
178191
data-test-subj="apmGroupDetailsCancelButton"
179192
onClick={onCloseModal}
193+
isLoading={isLoading}
180194
isDisabled={isLoading}
181195
>
182196
{i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.cancel', {
@@ -192,12 +206,13 @@ export function GroupDetails({
192206
iconSide="right"
193207
onClick={() => {
194208
onClickNext({
195-
groupName: name,
209+
groupName: name || '',
196210
color,
197211
description,
198212
kuery: serviceGroup?.kuery ?? '',
199213
});
200214
}}
215+
isLoading={isLoading}
201216
isDisabled={isInvalid || isLoading}
202217
>
203218
{i18n.translate('xpack.apm.serviceGroups.groupDetailsForm.selectServices', {

‎x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/save_modal.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* 2.0; you may not use this file except in compliance with the Elastic License
55
* 2.0.
66
*/
7-
import { EuiModal } from '@elastic/eui';
7+
import { EuiModal, useGeneratedHtmlId } from '@elastic/eui';
88
import { i18n } from '@kbn/i18n';
99
import { useHistory } from 'react-router-dom';
1010
import React, { useCallback, useEffect, useState } from 'react';
@@ -89,6 +89,8 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) {
8989
[savedServiceGroup?.id, notifications.toasts, onClose, isEdit, navigateToServiceGroups]
9090
);
9191

92+
const modalTitleId = useGeneratedHtmlId();
93+
9294
const onDelete = useCallback(
9395
async function () {
9496
setIsLoading(true);
@@ -115,7 +117,7 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) {
115117
);
116118

117119
return (
118-
<EuiModal onClose={onClose}>
120+
<EuiModal onClose={onClose} aria-labelledby={modalTitleId}>
119121
{modalView === 'group_details' && (
120122
<GroupDetails
121123
serviceGroup={stagedServiceGroup}
@@ -127,6 +129,7 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) {
127129
}}
128130
onDeleteGroup={onDelete}
129131
isLoading={isLoading}
132+
titleId={modalTitleId}
130133
/>
131134
)}
132135
{modalView === 'select_service' && stagedServiceGroup && (
@@ -139,6 +142,7 @@ export function SaveGroupModal({ onClose, savedServiceGroup }: Props) {
139142
setModalView('group_details');
140143
}}
141144
isLoading={isLoading}
145+
titleId={modalTitleId}
142146
/>
143147
)}
144148
</EuiModal>

‎x-pack/plugins/observability_solution/apm/public/components/app/service_groups/service_group_save/select_services.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ interface Props {
5454
onSaveClick: (serviceGroup: StagedServiceGroup) => void;
5555
onEditGroupDetailsClick: () => void;
5656
isLoading: boolean;
57+
titleId?: string;
5758
}
5859

5960
export function SelectServices({
@@ -63,6 +64,7 @@ export function SelectServices({
6364
onSaveClick,
6465
onEditGroupDetailsClick,
6566
isLoading,
67+
titleId,
6668
}: Props) {
6769
const [kuery, setKuery] = useState(serviceGroup?.kuery || '');
6870
const [stagedKuery, setStagedKuery] = useState(serviceGroup?.kuery || '');
@@ -117,7 +119,7 @@ export function SelectServices({
117119
<Container>
118120
<EuiModalHeader>
119121
<div>
120-
<EuiModalHeaderTitle>
122+
<EuiModalHeaderTitle id={titleId}>
121123
{i18n.translate('xpack.apm.serviceGroups.selectServicesForm.title', {
122124
defaultMessage: 'Select services',
123125
})}

0 commit comments

Comments
 (0)