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

[Controls] Use Trigger / Actions Framework for Hover #143585

Closed
Tracked by #144752
ThomThomson opened this issue Oct 18, 2022 · 1 comment · Fixed by #153065
Closed
Tracked by #144752

[Controls] Use Trigger / Actions Framework for Hover #143585

ThomThomson opened this issue Oct 18, 2022 · 1 comment · Fixed by #153065
Assignees
Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@ThomThomson
Copy link
Contributor

The Controls have icon actions which show up when the user hovers their mouse. These actions are currently not based on the actions / triggers framework. This worked fine for the first release, due to the fact that every control type had two actions, edit and remove but now that we are planning on adding new actions that appear depending on the control type, or field, we should think about using a more generic system for running these actions.

That system already exists, in the form of embeddable triggers / actions.

@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:large Large Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Project:Controls labels Oct 18, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

Heenawter added a commit that referenced this issue Nov 24, 2022
…144867)

Closes #140174
Closes #145040
Closes #146086 

## Summary

This PR adds two features to the options list control:
1. A button in the options list popover that gives users the ability to
change how the suggestions are sorted
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203416853-58f9c909-8909-4902-adf3-59831018c96f.gif"/>
    </p>

2. A per-control setting that disables the ability to dynamically sort
which, if set to `false`, presents the author with the ability to select
one of the four sorting methods for that specific control to use
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203417193-cd35f264-8c29-4c80-b88b-15da25a1f56c.gif"/>
    </p>

### Design considerations
@elastic/kibana-design 

As noted by Andrea when looking at the preliminary behaviour of this
feature, the `"Show only selected"` toggle has increased in importance
because of the new sorting mechanic - after all, when making selections
and then changing the sort method, your selections can appear to be
"lost" if you have enough unique values in the control's field.

In the original designs, the `"Clear all selections"` button was
**first** in the popover's action bar - however, I found that I kept
accidentally clicking this in my testing when switching between
searching, sorting, making selections, changing sorting, showing only
selected options, etc. etc. I found that the following design felt a lot
more natural for the placement of the `"Clear all selections"` button:


![image](https://user-images.githubusercontent.com/8698078/202318768-cf8a5668-40c4-482f-9eb0-023508866068.png)

Note that, once #143585 is
resolved, this will no longer be as much of a concern because we will be
moving, at the very least, the `"Clear all selections"` to be a floating
action. That being said, this new order for the actions is, in my
opinion, a good compromise in the mean time. Very much open to feedback,
though!

### Video 


https://user-images.githubusercontent.com/8698078/203422674-52aac87c-7295-4eb6-99a5-ee3ffba2756b.mov


### Testing Notes
There are a few things to consider when testing:
1. Does the dynamic sorting give you expected results when sorting
various field types?
- Note that IP fields only support document count sorting, so ensure
that "Alphabetical" sorting does not show up in the sorting list during
either creation or as part of the popover sorting.
2. When setting the `"Allow suggestions to be sorted"` toggle to
`false`, it should always default to `"Document count (descending)"` to
prevent invalid sort selections. For example, consider the following:
    - Create an options list control on some keyword field
- Set the sort to alphabetical (either ascending or descending) in the
popover
    - Edit that control and change it to an IP field
    - Set `"Allow suggestions to be sorted"` to `false
- The default sort should be `"Document count (descending)"` and **not**
`"Alphabetical (descending/ascending)"`, since alphabetical sorting
would be invalid in this case.

**Flaky Test Runner**
<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1585"><img
src="https://user-images.githubusercontent.com/8698078/203428246-13f5ff9a-df0c-4cd5-a4ee-cf7a98792362.png"/></a>



### Checklist

Delete any items that are not applicable to this PR.

- [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] 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))
<p><img
src="https://user-images.githubusercontent.com/8698078/202545715-96daa0ab-8900-45cb-979f-20a83e622597.png"/></p>
- [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>
@Heenawter Heenawter self-assigned this Feb 7, 2023
Heenawter added a commit that referenced this issue Mar 21, 2023
…3065)

Closes #143585
Closes #151767
Closes #152609

## Summary
This PR accomplishes three things, the first of which is moving the
edit/delete control hover actions to use the `uiActions` service - this
is the first step in moving existing panel actions (such as replacing
the panel, opening the panel settings flyout, etc.) to this hover
framework, which is outlined in
[this](#151233) issue.

While this was the primary goal of this PR, this also made the following
fixes possible:
1. Since I was refactoring the control editor flyout code as part of
this PR, I made it so that changes to the control's width/grow
properties are **only applied** when the changes are **saved** rather
than being automatically applied.

    | Before            | After |
    | ------------- | ------------- |
| ![Mar-14-2023
13-05-36](https://user-images.githubusercontent.com/8698078/225110954-d443f76b-4ac7-476c-b5b7-9af082d187fd.gif)
| ![Mar-14-2023
13-06-41](https://user-images.githubusercontent.com/8698078/225111172-ab9cce7e-7a70-45e4-ab06-5a87c053fb95.gif)
|
  


2. Since the edit control button is no longer a custom component, the
tooltip now responds to focus as expected.

    | Before            | After |
    | ------------- | ------------- |
| ![Mar-14-2023
13-05-36](https://user-images.githubusercontent.com/8698078/225113458-fe8f05fb-d56c-437a-b625-2a336bb4ba29.gif)
| ![Mar-14-2023
13-06-41](https://user-images.githubusercontent.com/8698078/225113313-d8cb7fcc-f611-48d0-83b4-f6fd147ce0ae.gif)
|


### 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 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
Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants