Skip to content
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

Add tooltips to Discover button icons #192963

Merged
merged 5 commits into from
Sep 17, 2024
Merged

Conversation

smith
Copy link
Contributor

@smith smith commented Sep 16, 2024

Summary

When navigating through elements in Discover with a keyboard, icon-only buttons don't show their browser-native title attribute when focused, only when hovered.

This makes it unclear when you "tab in to" an icon-only button what the button does.

These changes add a tooltip on the filter in/out buttons on the field list values.

Before After
CleanShot 2024-09-15 at 21 20 36@2x CleanShot 2024-09-15 at 21 17 23@2x

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@smith smith marked this pull request as ready for review September 16, 2024 02:30
@smith smith requested review from a team as code owners September 16, 2024 02:30
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Hi @smith,

Thanks for adding tooltips! Unfortunately, looks like it affected the layout of toolbar icons. Would be great to address it before merging.

Screenshot 2024-09-16 at 12 33 01 (in your screenshots too)

@smith
Copy link
Contributor Author

smith commented Sep 16, 2024

@jughosta thanks I totally missed that. It looks like there's some custom styles that break when you add tooltips to these buttons. I'll leave this open for now and pick it up in the future.

@smith smith requested a review from jughosta September 17, 2024 02:28
@smith
Copy link
Contributor Author

smith commented Sep 17, 2024

@jughosta I updated this to only change the field list filter items.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #13 / SyncAlertsSwitch it toggles the switch

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
aiops 547.7KB 547.8KB +88.0B
apm 3.5MB 3.5MB +88.0B
discover 812.7KB 812.8KB +88.0B
lens 1.5MB 1.5MB +88.0B
ml 4.6MB 4.6MB +88.0B
securitySolution 20.4MB 20.4MB +88.0B
slo 852.9KB 853.0KB +88.0B
total +616.0B

History

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

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kertal kertal added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Sep 17, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@smith smith merged commit c12c361 into elastic:main Sep 17, 2024
22 checks passed
@kibanamachine kibanamachine added v9.0.0 backport:skip This commit does not require backporting labels Sep 17, 2024
@davismcphee
Copy link
Contributor

@smith Should this have a backport:prev-minor label on it to ensure it gets backported to 8.16 and not just 9.0?

@smith smith added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Sep 27, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 27, 2024
## Summary

When navigating through elements in Discover with a keyboard, icon-only
buttons don't show their browser-native title attribute when focused,
only when hovered.

This makes it unclear when you "tab in to" an icon-only button what the
button does.

These changes add a tooltip on the filter in/out buttons on the field
list values.

|    Before    |    After     |
| :----------: | :----------: |
| ![CleanShot 2024-09-15 at 21 20
36@2x](https://github.com/user-attachments/assets/ed915d78-fc66-4eb4-a4d7-d7c3395edfc6)
|![CleanShot 2024-09-15 at 21 17
23@2x](https://github.com/user-attachments/assets/0d7b9862-b8ab-4a2c-bc4b-8b790c393001)
|

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

(cherry picked from commit c12c361)
@kibanamachine
Copy link
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 Sep 27, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Add tooltips to Discover button icons
(#192963)](#192963)

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

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

<!--BACKPORT [{"author":{"name":"Nathan L
Smith","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-17T21:32:33Z","message":"Add
tooltips to Discover button icons (#192963)\n\n## Summary\r\n\r\nWhen
navigating through elements in Discover with a keyboard,
icon-only\r\nbuttons don't show their browser-native title attribute
when focused,\r\nonly when hovered.\r\n\r\nThis makes it unclear when
you \"tab in to\" an icon-only button what the\r\nbutton
does.\r\n\r\nThese changes add a tooltip on the filter in/out buttons on
the field\r\nlist values.\r\n\r\n| Before | After |\r\n| :----------: |
:----------: |\r\n| ![CleanShot 2024-09-15 at 21
20\r\n36@2x](https://github.com/user-attachments/assets/ed915d78-fc66-4eb4-a4d7-d7c3395edfc6)\r\n|![CleanShot
2024-09-15 at 21
17\r\n23@2x](https://github.com/user-attachments/assets/0d7b9862-b8ab-4a2c-bc4b-8b790c393001)\r\n|\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))","sha":"c12c361f5636791999345bc119b60139976bdd13","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:fix","v9.0.0","Team:DataDiscovery","backport:prev-minor"],"title":"Add
tooltips to Discover button
icons","number":192963,"url":"https://github.com/elastic/kibana/pull/192963","mergeCommit":{"message":"Add
tooltips to Discover button icons (#192963)\n\n## Summary\r\n\r\nWhen
navigating through elements in Discover with a keyboard,
icon-only\r\nbuttons don't show their browser-native title attribute
when focused,\r\nonly when hovered.\r\n\r\nThis makes it unclear when
you \"tab in to\" an icon-only button what the\r\nbutton
does.\r\n\r\nThese changes add a tooltip on the filter in/out buttons on
the field\r\nlist values.\r\n\r\n| Before | After |\r\n| :----------: |
:----------: |\r\n| ![CleanShot 2024-09-15 at 21
20\r\n36@2x](https://github.com/user-attachments/assets/ed915d78-fc66-4eb4-a4d7-d7c3395edfc6)\r\n|![CleanShot
2024-09-15 at 21
17\r\n23@2x](https://github.com/user-attachments/assets/0d7b9862-b8ab-4a2c-bc4b-8b790c393001)\r\n|\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))","sha":"c12c361f5636791999345bc119b60139976bdd13"}},"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/192963","number":192963,"mergeCommit":{"message":"Add
tooltips to Discover button icons (#192963)\n\n## Summary\r\n\r\nWhen
navigating through elements in Discover with a keyboard,
icon-only\r\nbuttons don't show their browser-native title attribute
when focused,\r\nonly when hovered.\r\n\r\nThis makes it unclear when
you \"tab in to\" an icon-only button what the\r\nbutton
does.\r\n\r\nThese changes add a tooltip on the filter in/out buttons on
the field\r\nlist values.\r\n\r\n| Before | After |\r\n| :----------: |
:----------: |\r\n| ![CleanShot 2024-09-15 at 21
20\r\n36@2x](https://github.com/user-attachments/assets/ed915d78-fc66-4eb4-a4d7-d7c3395edfc6)\r\n|![CleanShot
2024-09-15 at 21
17\r\n23@2x](https://github.com/user-attachments/assets/0d7b9862-b8ab-4a2c-bc4b-8b790c393001)\r\n|\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))","sha":"c12c361f5636791999345bc119b60139976bdd13"}}]}]
BACKPORT-->

Co-authored-by: Nathan L Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants