-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Uptime] Improve filter group #88185
[Uptime] Improve filter group #88185
Conversation
a76cac1
to
452882c
Compare
452882c
to
0d27256
Compare
Pinging @elastic/uptime (Team:uptime) |
schemes: [], | ||
locations: ['nyc', 'fairbanks'], | ||
}, | ||
filterAriaLabels.TAG, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will it be filterAriaLabels.TAG
in all cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I misunderstood your ask. I will address this.
if (isOpen) { | ||
// only update these values on close | ||
onFilterFieldChange(fieldName, tempSelectedItems); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to RTL 🚀
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@shahzad31 please have a look at 8fb99f5 and re-approve if it looks good to you. Thanks! |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚢
* Unskip "Observer location" test block. * Commit temp "describe.only" to make flaky test runner go faster. * Add optional chain for some potentially-null props. * Make overview filters type partial. * Repair broken types. * Remove only call from test. * Add unit tests and mark areas for improvement in \`FilterGroup\` component. * Add aria-label translations and new labels. * Refactor existing tests and add tests for new labels. * Fix bug in event handler and update tests. * Delete a comment. * Delete a comment. * Add some line breaks to help readability. * Add additional tests, fix a bug. Co-authored-by: Kibana Machine <[email protected]>
* Unskip "Observer location" test block. * Commit temp "describe.only" to make flaky test runner go faster. * Add optional chain for some potentially-null props. * Make overview filters type partial. * Repair broken types. * Remove only call from test. * Add unit tests and mark areas for improvement in \`FilterGroup\` component. * Add aria-label translations and new labels. * Refactor existing tests and add tests for new labels. * Fix bug in event handler and update tests. * Delete a comment. * Delete a comment. * Add some line breaks to help readability. * Add additional tests, fix a bug. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
* Unskip "Observer location" test block. * Commit temp "describe.only" to make flaky test runner go faster. * Add optional chain for some potentially-null props. * Make overview filters type partial. * Repair broken types. * Remove only call from test. * Add unit tests and mark areas for improvement in \`FilterGroup\` component. * Add aria-label translations and new labels. * Refactor existing tests and add tests for new labels. * Fix bug in event handler and update tests. * Delete a comment. * Delete a comment. * Add some line breaks to help readability. * Add additional tests, fix a bug. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Backported to: |
Summary
note to reviewers, do not review until #87571 is mergedResolves #88180.
Adds unit tests for the
FilterGroupComponent
to ensure appropriate text is displayed during loading and when items are present, and ensures the popover functionality displays filter items.An additional focus of this PR is improving a11y concerns while this code is being touched for test add. Improving the state of
aria-label
s not only improves the UX for users that rely on these attributes, it makes testing easier as well.Checklist
Delete any items that are not applicable to this PR.
For maintainers