[Controls] Make "clear selections" a hover action for all data controls #159395
Assignees
Labels
enhancement
New value added to drive a business result
Feature:Input Control
Input controls visualization
impact:high
Addressing this issue will have a high level of impact on the quality/strength of our product.
loe:medium
Medium Level of Effort
Project:Controls
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
Comments
Heenawter
added
Feature:Input Control
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
|
➕ ➕ on this one because it is required for range slider stability which falls into our controls stability bucket for 8.9. |
|
Should this also include time slider control? |
|
Looks like it would be pretty trivial to make the hover action apply to the time slider as well. |
This was referenced Jun 12, 2023
Heenawter
added a commit
that referenced
this issue
Jun 15, 2023
Closes #135466 ## Summary The main goal of this PR is to fix the serious "Buttons must have discernible text" a11y failure - this is accomplished by switching from building our own range slider button using `EuiFlexGroup` to instead using `EuiFormControlLayoutDelimited`, which both resolves these a11y issues and also fixes a rendering regression: | Before | After | |--------|-------| |  |  | As part of this, I also took some time to clean up some of the range slider code, which hasn't really been touched in awhile - this includes... - moving the debounce on range selections from the embeddable's `input$` subscription to the component itself, as described [here](#159271 (comment)). - fixing a bug where resetting the range slider would unnecessarily cause unsaved changes, as described [here](#159271 (comment)). - improving the `onClick` behaviour (with some notable limitations), as described [here](#159271 (comment)). As a follow up, we need to move the "clear selections" button [to a hover action](#159395), which will enable us to then fully move forward with our transition to the `EuiDualRange` component. ### 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 - [ ] ~Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))~ > **Note** > Details provided [here](#159271 (comment)) on why only partial keyboard support is currently supported - [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)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Heenawter
added a commit
that referenced
this issue
Jun 20, 2023
Closes #159395 Closes #153383 ## Summary This PR moves the "clear selections" button for all controls (options list, range slider, and time slider) from inside their respective popovers to a general hover action - this not only saves users a click for this common interaction (which has actually been brought in user feedback up as a downside of the current controls compared to the legacy controls), it also allows us to fully move forward with migrating the range slider control to the `EuiDualRange` component. This will be done in a follow up PR, which should both (1) clean up our range slider code significantly and (2) fix the [bug discussed here](#159271 (review)). The related issue can be tracked [here](#159724), since we might not be able to get to it right away. This "clear selections" action is available in both view and edit mode, like so: | | Edit mode | View mode | |--------|--------|--------| | **Range slider** |  |  | | **Options list** |  |  | | **Time slider** |  |  | You may notice in the above screenshots that the "delete" action is now represented with a red trash icon rather than a red cross, and the tooltip text was also changed to use the word "Delete" rather than the word "Remove" - these changes were both made to be more consistent with the "Delete panel" action available on dashboards: | Delete control - Before | Delete control - After | Delete panel | |--------|--------|--------| |  |  | <img src="https://github.com/elastic/kibana/assets/8698078/a7f65777-45cf-44f2-96a7-f1042cb25e02"/> | Beyond these changes, I also made a few quick changes to the time slider control, including: 1. Fixing the appearance so that the background is once again white, as described [here](#159526 (comment)) 2. Adding comparison logic so that clearing selections no longer causes unsaved changes unnecessarily, as described [here](#159526 (comment)) ### Videos **Before** https://github.com/elastic/kibana/assets/8698078/96365c85-748e-4fd7-ae5d-589aa11a23ef **After** https://github.com/elastic/kibana/assets/8698078/68352559-e71b-4b5e-8709-587016f0b35a ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the feature:
As a follow up to #153065 and #159271, in order to fully make the switch to the
EuiDualRangecomponent, we need to remove any unnecessary stuff from the range slider popover - this includes the "Clear selections" button. So, by creating a newuiActionsframework hover action, this has the benefit of both:EuiDualRangecomponent, which will clean up our code significantly and should also have performance + stability improvements, andThe text was updated successfully, but these errors were encountered: