fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active Now elems must both be keyboard focsuable #186529

alexwizp · 2024-06-20T13:28:49Z

Closes: https://github.com/elastic/observability-dev/issues/3371

Description

The Obs Alert Rule Detail view has a card that is clickable with a focusable element inside it. This is a confusing paradigm and prevents keyboard users from filtering by all alerts because it's not focusable. It would be better to make the two alert number widgets the focusable elements. Screenshot attached below.

PR is based on the following comment posted by @1Copenut in https://github.com/elastic/observability-dev/issues/3371#issuecomment-2129446431_

@alexwizp Agreed, panels should not be focusable. The highlighted panel is clickable, and that was unexpected. I could click the entire panel, and click the "1 Active now" text to filter by all alerts or active alerts in the table below.

It would be better to have the "All alerts" text be clickable and focusable, and keep the "1 Active now" clickable and focusable. That way the two text blocks have the interactive behavior, while the panel (card) is just a container.

Steps to recreate

Open the Obs Alerts table
Click the "Manage Rules" link
Create a new rule and verify it appears in the Rules table
Click on the rule name to load the Rule Detail view
Verify the 1 Active Now

What was done?:

The click event was REMOVED from the panel and has been moved to All alerts.
aria-describedby attributes were added for AllAlertCounts and ActiveAlertCounts
h3 attributes were replaced to EuiTitle in AllAlertCounts and ActiveAlertCounts

… Now elems must both be keyboard focsuable Closes: elastic/observability-dev#3371

alexwizp · 2024-06-20T13:28:56Z

/ci

alexwizp · 2024-06-20T15:42:22Z

/ci

alexwizp · 2024-06-20T20:30:27Z

/ci

alexwizp · 2024-06-21T06:44:12Z

/ci

elasticmachine · 2024-06-21T08:32:28Z

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

maryam-saeidi · 2024-06-21T09:34:44Z

The Obs Alert Rule Detail view has a card that is clickable with a focusable element inside it. This is a confusing paradigm and prevents keyboard users from filtering by all alerts because it's not focusable. It would be better to make the two alert number widgets the focusable elements

This implementation was based on the original design by @maciejforcone.
This PR removes the ability to click the panel and instead adds the click functionality to the 1 Alerts, so I tag our designer, @maciejforcone, here to get his feedback.

maciejforcone · 2024-06-21T09:56:04Z

This implementation was based on the original design by @maciejforcone. This PR removes the ability to click the panel and instead adds the click functionality to the 1 Alerts, so I tag our designer, @maciejforcone, here to get his feedback.

I think the problem here is that this was a widget to place in other observability areas, like service details, host details, etc, in compact format, almost like button size. In rule it's a bit different story.
So for consistency, let's stick with clickability of text only, and no shadowbox (for entire section), so that user is not missing anything.

maryam-saeidi

Code LGTM, I've just added a nit. Thanks for fixing this!

I am going to test locally now.

...tions_ui/public/application/sections/alert_summary_widget/components/active_alert_counts.tsx

maryam-saeidi · 2024-06-21T10:17:48Z

...gers_actions_ui/public/application/sections/alert_summary_widget/components/alert_counts.tsx

    event: MouseEvent<HTMLAnchorElement | HTMLDivElement>,
    status?: AlertStatus
  ) => void;
 }

-export const AlertCounts = ({ activeAlertCount, recoveredAlertCount, onActiveClick }: Props) => {
+/** @internal **/
+const AlertItem = ({


nit: What about moving this logic to a separate file and then using it directly in AllAlertCounts and ActiveAlertCounts?

In that case, we can also move onAllClick and onActiveClick to the respective components.
In the current implementation, we have some parts of the allAlerts/activeAlerts logic here and some parts of it in the children components.

So, in the end, the component here will look something like:

<EuiFlexGroup gutterSize="l" responsive={false}> <AllAlertCounts count={activeAlertCount + recoveredAlertCount} onClick={onAllClick} data-test-subj="allAlerts" /> <ActiveAlertCounts activeAlertCount={activeAlertCount} onClick={onActiveClick} data-test-subj="activeAlerts" /> </EuiFlexGroup>

@maryam-saeidi, could you please review the latest changes? I'm unsure if we need both AllAlertCounts and ActiveAlertCounts. I've decided to retain only AlertItem to remove duplication.

maryam-saeidi

Tested locally and worked as expected!

alexwizp · 2024-07-08T07:05:54Z

@elastic/response-ops please review

alexwizp · 2024-07-11T14:40:07Z

@elastic/response-ops please review

alexwizp · 2024-07-15T06:54:39Z

@elastic/response-ops please review

alexwizp · 2024-07-17T15:29:14Z

@elastic/response-ops please review

elasticmachine · 2024-07-19T09:49:53Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 9e868ac
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-186529-9e868ac8cd6b
Observability Deployment

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`triggersActionsUi`	803	802	-1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`triggersActionsUi`	120.4KB	120.3KB	-135.0B

History

💛 Build #221969 was flaky ea7ae9c
💚 Build #221317 succeeded e6fa7bd
💛 Build #220957 was flaky 7798234
💔 Build #220916 failed ccf4f0f

kibana-ci · 2024-07-24T15:51:45Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7774ba8
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-186529-7774ba8fed2a
Observability Deployment

Failed CI Steps

FTR Configs #100

Test Failures

[job] [logs] FTR Configs #100 / Screenshots - serverless observability UI response ops docs observability cases Observability case settings case settings screenshots

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`triggersActionsUi`	803	802	-1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`triggersActionsUi`	120.4KB	120.3KB	-135.0B

History

💔 Build #223478 failed 408a3af
💔 Build #223444 failed 87b4818
💛 Build #223413 was flaky 62d8c64
💛 Build #222835 was flaky 828a6d7

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

adcoelho

Tested and works as expected 👍

fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active…

49b026c

… Now elems must both be keyboard focsuable Closes: elastic/observability-dev#3371

fix ts issues

564f60b

fix tests

60ca3ca

fix tests

cceb364

alexwizp added Project:Accessibility v8.15.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Jun 21, 2024

alexwizp marked this pull request as ready for review June 21, 2024 08:32

alexwizp requested review from a team as code owners June 21, 2024 08:32

Merge branch 'main' into elastic#3371

2c2185e

botelastic bot added the ci:project-deploy-observability Create an Observability project label Jun 21, 2024

maryam-saeidi reviewed Jun 21, 2024

View reviewed changes

maryam-saeidi approved these changes Jun 21, 2024

View reviewed changes

alexwizp added 8 commits June 25, 2024 13:47

Merge remote-tracking branch 'upstream/main' into elastic#3371

5726099

update PR

31274c4

fix functional tests

33f10e7

Merge branch 'main' into elastic#3371

0d833a7

Merge branch 'main' into elastic#3371

9d8f9a0

Merge branch 'main' into elastic#3371

5d7a8cb

Merge branch 'main' into elastic#3371

9614806

Merge branch 'main' into elastic#3371

db53e47

alexwizp added v8.16.0 and removed v8.15.0 labels Jul 8, 2024

alexwizp added 2 commits July 9, 2024 10:10

Merge branch 'main' into elastic#3371

9101739

Merge branch 'main' into elastic#3371

de54c05

alexwizp added 3 commits July 11, 2024 17:40

Merge branch 'main' into elastic#3371

ccf4f0f

Merge branch 'main' into elastic#3371

7798234

Merge branch 'main' into elastic#3371

e6fa7bd

Merge branch 'main' into elastic#3371

ea7ae9c

Merge branch 'main' into elastic#3371

9e868ac

alexwizp added 5 commits July 22, 2024 12:12

Merge branch 'main' into elastic#3371

828a6d7

Merge branch 'main' into elastic#3371

62d8c64

Merge branch 'main' into elastic#3371

87b4818

Merge branch 'main' into elastic#3371

408a3af

Merge branch 'main' into elastic#3371

7774ba8

adcoelho approved these changes Jul 25, 2024

View reviewed changes

alexwizp merged commit 631baa3 into elastic:main Jul 25, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active Now elems must both be keyboard focsuable #186529

fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active Now elems must both be keyboard focsuable #186529

alexwizp commented Jun 20, 2024 •

edited

Loading

alexwizp commented Jun 20, 2024

alexwizp commented Jun 20, 2024

alexwizp commented Jun 20, 2024

alexwizp commented Jun 21, 2024

elasticmachine commented Jun 21, 2024

maryam-saeidi commented Jun 21, 2024

maciejforcone commented Jun 21, 2024

maryam-saeidi left a comment

maryam-saeidi Jun 21, 2024 •

edited

Loading

alexwizp Jun 25, 2024

maryam-saeidi left a comment

alexwizp commented Jul 8, 2024

alexwizp commented Jul 11, 2024

alexwizp commented Jul 15, 2024

alexwizp commented Jul 17, 2024

elasticmachine commented Jul 19, 2024 •

edited

Loading

kibana-ci commented Jul 24, 2024 •

edited

Loading

adcoelho left a comment

fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active Now elems must both be keyboard focsuable #186529

fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active Now elems must both be keyboard focsuable #186529

Conversation

alexwizp commented Jun 20, 2024 • edited Loading

Description

Steps to recreate

What was done?:

alexwizp commented Jun 20, 2024

alexwizp commented Jun 20, 2024

alexwizp commented Jun 20, 2024

alexwizp commented Jun 21, 2024

elasticmachine commented Jun 21, 2024

maryam-saeidi commented Jun 21, 2024

maciejforcone commented Jun 21, 2024

maryam-saeidi left a comment

Choose a reason for hiding this comment

maryam-saeidi Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

alexwizp Jun 25, 2024

Choose a reason for hiding this comment

maryam-saeidi left a comment

Choose a reason for hiding this comment

alexwizp commented Jul 8, 2024

alexwizp commented Jul 11, 2024

alexwizp commented Jul 15, 2024

alexwizp commented Jul 17, 2024

elasticmachine commented Jul 19, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Page load bundle

History

kibana-ci commented Jul 24, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Page load bundle

History

adcoelho left a comment

Choose a reason for hiding this comment

alexwizp commented Jun 20, 2024 •

edited

Loading

maryam-saeidi Jun 21, 2024 •

edited

Loading

elasticmachine commented Jul 19, 2024 •

edited

Loading

kibana-ci commented Jul 24, 2024 •

edited

Loading