Skip to content

fix: [Obs Services > Create Service Group modal][KEYBOARD]: Focus should not be auto-set on first input when modal appears#194696

Merged
alexwizp merged 8 commits intoelastic:mainfrom
alexwizp:issues/26
Oct 7, 2024
Merged

fix: [Obs Services > Create Service Group modal][KEYBOARD]: Focus should not be auto-set on first input when modal appears#194696
alexwizp merged 8 commits intoelastic:mainfrom
alexwizp:issues/26

Conversation

@alexwizp
Copy link
Copy Markdown
Contributor

@alexwizp alexwizp commented Oct 2, 2024

Closes: #194965
Closes: #194966

Description

Changes Made

  1. Removed:
- inputRef.current?.focus(); // autofocus on initial render
  1. Added aria-labelledby={modalTitleId} for EuiModal. See https://eui.elastic.co/#/layout/modal.
  2. Slightly updated Name and Color validation.

Screen

Screen.Recording.2024-10-02.at.15.32.49.mov

@alexwizp
Copy link
Copy Markdown
Contributor Author

alexwizp commented Oct 2, 2024

/ci

@alexwizp
Copy link
Copy Markdown
Contributor Author

alexwizp commented Oct 2, 2024

/ci

@alexwizp alexwizp marked this pull request as ready for review October 2, 2024 15:45
@alexwizp alexwizp requested a review from a team October 2, 2024 15:45
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Oct 2, 2024
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Oct 7, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 13b8b63
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-194696-13b8b635b417

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.4MB 3.4MB +251.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@alexwizp alexwizp merged commit d576365 into elastic:main Oct 7, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11215089937

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 7, 2024
…uld not be auto-set on first input when modal appears (elastic#194696)

Closes: elastic#194965
Closes: elastic#194966

# Description

- [x] elastic#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] elastic#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)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 7, 2024
…Focus should not be auto-set on first input when modal appears (#194696) (#195235)

# Backport

This will backport the following commits from `main` to `8.x`:
- [fix: [Obs Services &gt; Create Service Group modal][KEYBOARD]: Focus
should not be auto-set on first input when modal appears
(#194696)](#194696)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alexey
Antonov","email":"alexwizp@gmail.com"},"sourceCommit":{"committedDate":"2024-10-07T12:04:39Z","message":"fix:
[Obs Services > Create Service Group modal][KEYBOARD]: Focus should not
be auto-set on first input when modal appears (#194696)\n\nCloses:
#194965 \r\nCloses:
https://github.com/elastic/kibana/issues/194966\r\n\r\n# Description
\r\n\r\n- [x] #194965 <br />
Focus is\r\ncurrently being set on the first input in the \"Create
group\" modal.\r\nScreen reader users will hear the input name, but not
get the title of\r\nthe modal read aloud this way, and it could be
confusing. We should be\r\nletting the EuiModal set focus naturally on
the modal or close button so\r\nscreen reader users hear the title as
expected.\r\n\r\n\r\n- [x]
#194966 <br /> Focus must\r\nbe
returned properly when I cancel the \"Create group\" workflow
in\r\nServices > Create service group modal.\r\n\r\n# Changes
Made\r\n\r\n1. Removed:\r\n\r\n```diff \r\n- inputRef.current?.focus();
// autofocus on initial render\r\n```\r\n\r\n2. Added
`aria-labelledby={modalTitleId}` for `EuiModal`.
See\r\nhttps://eui.elastic.co/#/layout/modal.\r\n3. Slightly updated
`Name` and `Color` validation.\r\n\r\n\r\n#
Screen\r\n\r\n\r\nhttps://github.com/user-attachments/assets/6636f2dc-b9b7-4d4d-8144-90249f8327e7","sha":"d5763658c39856aefb5e15fa9e3e771f8bb0d613","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Project:Accessibility","release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","apm:review"],"title":"fix:
[Obs Services > Create Service Group modal][KEYBOARD]: Focus should not
be auto-set on first input when modal
appears","number":194696,"url":"https://github.com/elastic/kibana/pull/194696","mergeCommit":{"message":"fix:
[Obs Services > Create Service Group modal][KEYBOARD]: Focus should not
be auto-set on first input when modal appears (#194696)\n\nCloses:
#194965 \r\nCloses:
https://github.com/elastic/kibana/issues/194966\r\n\r\n# Description
\r\n\r\n- [x] #194965 <br />
Focus is\r\ncurrently being set on the first input in the \"Create
group\" modal.\r\nScreen reader users will hear the input name, but not
get the title of\r\nthe modal read aloud this way, and it could be
confusing. We should be\r\nletting the EuiModal set focus naturally on
the modal or close button so\r\nscreen reader users hear the title as
expected.\r\n\r\n\r\n- [x]
#194966 <br /> Focus must\r\nbe
returned properly when I cancel the \"Create group\" workflow
in\r\nServices > Create service group modal.\r\n\r\n# Changes
Made\r\n\r\n1. Removed:\r\n\r\n```diff \r\n- inputRef.current?.focus();
// autofocus on initial render\r\n```\r\n\r\n2. Added
`aria-labelledby={modalTitleId}` for `EuiModal`.
See\r\nhttps://eui.elastic.co/#/layout/modal.\r\n3. Slightly updated
`Name` and `Color` validation.\r\n\r\n\r\n#
Screen\r\n\r\n\r\nhttps://github.com/user-attachments/assets/6636f2dc-b9b7-4d4d-8144-90249f8327e7","sha":"d5763658c39856aefb5e15fa9e3e771f8bb0d613"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194696","number":194696,"mergeCommit":{"message":"fix:
[Obs Services > Create Service Group modal][KEYBOARD]: Focus should not
be auto-set on first input when modal appears (#194696)\n\nCloses:
#194965 \r\nCloses:
https://github.com/elastic/kibana/issues/194966\r\n\r\n# Description
\r\n\r\n- [x] #194965 <br />
Focus is\r\ncurrently being set on the first input in the \"Create
group\" modal.\r\nScreen reader users will hear the input name, but not
get the title of\r\nthe modal read aloud this way, and it could be
confusing. We should be\r\nletting the EuiModal set focus naturally on
the modal or close button so\r\nscreen reader users hear the title as
expected.\r\n\r\n\r\n- [x]
#194966 <br /> Focus must\r\nbe
returned properly when I cancel the \"Create group\" workflow
in\r\nServices > Create service group modal.\r\n\r\n# Changes
Made\r\n\r\n1. Removed:\r\n\r\n```diff \r\n- inputRef.current?.focus();
// autofocus on initial render\r\n```\r\n\r\n2. Added
`aria-labelledby={modalTitleId}` for `EuiModal`.
See\r\nhttps://eui.elastic.co/#/layout/modal.\r\n3. Slightly updated
`Name` and `Color` validation.\r\n\r\n\r\n#
Screen\r\n\r\n\r\nhttps://github.com/user-attachments/assets/6636f2dc-b9b7-4d4d-8144-90249f8327e7","sha":"d5763658c39856aefb5e15fa9e3e771f8bb0d613"}}]}]
BACKPORT-->

Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apm:review ci:project-deploy-observability Create an Observability project Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. v8.16.0 v9.0.0

Projects

None yet

6 participants