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] Options List Add Alphabetical Sorting #140174

Closed
teresaalvarezsoler opened this issue Sep 7, 2022 · 5 comments · Fixed by #144867
Closed

[Controls] Options List Add Alphabetical Sorting #140174

teresaalvarezsoler opened this issue Sep 7, 2022 · 5 comments · Fixed by #144867
Assignees
Labels
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:medium Medium Level of Effort Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@teresaalvarezsoler
Copy link

teresaalvarezsoler commented Sep 7, 2022

Describe the feature:
Currently control values are sorted by cardinality in descending order (values with more documents first) but users would like to choose sorting values alphabetically. We will add a configuration option where users can select either alphabetical order or number of documents.

Needs design: @andreadelrio

@teresaalvarezsoler teresaalvarezsoler added Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Sep 7, 2022
@elasticmachine
Copy link
Contributor

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

@slhck

This comment was marked as outdated.

@teresaalvarezsoler
Copy link
Author

Sorry @slhck my bad linking the wrong github issue, here it is the right one: #140175

@ThomThomson ThomThomson changed the title [Controls] Add sorting capabilities to Controls [Controls] Options List Add Alphabetical Sorting Oct 18, 2022
@andreadelrio
Copy link
Contributor

First pass of how this could look like on the dropdown (starting to make more sense and more sense to move the eraser from that section too).

controls_sorting

@Heenawter
Copy link
Contributor

@andreadelrio This looks great! Love the different sort options.

++ for moving the erase button out in to a floating action ASAP - the "Sort" and the "Show only selected" buttons go together really well, but the erase button is feeling a bit... out of place now, haha! :)

@Heenawter Heenawter self-assigned this Nov 8, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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:medium Medium 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.

6 participants