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 Redux & Selection Management #115122

Merged

Conversation

ThomThomson
Copy link
Contributor

fixes #113489

Summary

This PR is a follow-up to #114371 which adds redux toolkit integration to the Options list control.

Additionally, this PR provides a couple user-facing changes

Options List "show selections only" mode
Show selections

Options List clear all button
Remove all selections

Removed Jumpy Loading
On each keystroke, when the typeahead would trigger, the loading indicator would show instead of the available options.
The loading indicator now shows over top of the available options, keeping the same size. This should reduce jumpiness when typing.
Screen Shot 2021-10-14 at 8 49 50 PM

Confirm on delete Control
Screen Shot 2021-10-14 at 8 47 15 PM

…ction clear buttons to options list. Options list clears selections when field or index pattern change
@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:medium Medium Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Project:Controls labels Oct 15, 2021
@ThomThomson ThomThomson marked this pull request as ready for review October 15, 2021 14:16
@ThomThomson ThomThomson requested review from a team as code owners October 15, 2021 14:16
@elasticmachine
Copy link
Contributor

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

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

@ThomThomson ThomThomson added the backport:skip This commit does not require backporting label Oct 18, 2021
singleSelect?: boolean;

indexPatternSelectOptions: Array<EuiSuperSelectOption<string>>;
availableIndexPatterns?: { [key: string]: IIndexPattern };
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily a thing to change but when do we wanna switch from IndexPattern stuff to DataView stuff? Is it just a 1-1 conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've switched over to the DataView format during the dashboard integration PR. #115477 is temporarily open to check for CI failures, you can take a look at the diff there!

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Code LGTM and works great!

Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

Looking good, just a couple of prop additions to some of the flex groups so that they behave better on smaller screens. End result:

image

@andreadelrio
Copy link
Contributor

You'll also need to add responsive={false} to the flex group inside EuiFlyoutFooters.

@ThomThomson
Copy link
Contributor Author

Added the flex group changes, good call, It looks a lot better in small sizes now!

@andreadelrio andreadelrio self-requested a review October 19, 2021 17:54
@ryankeairns ryankeairns self-requested a review October 19, 2021 20:06
@andreadelrio
Copy link
Contributor

andreadelrio commented Oct 20, 2021

@ryankeairns I tested how a subdued panel would look like for the empty state, like we discussed. The subdued color is actually the same background color as Kibana so there's no contrast which isn't something bad per se. What do you think?

subdued-emptyState

Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

Approving now as we will have a follow-up PR to continue refining the design side of this.

@ThomThomson
Copy link
Contributor Author

@andreadelrio @ryankeairns, I do like the design of the panel without a background color. In the dashboard implementation PR, I was also experimenting with removing the background by setting its color to transparent, and removing the drop shadow - as I think it adds a bit too much noise, and each individual control already has a panel. Additionally, without the background, I found that controls can line up much better with the dashboard panels without a panel wrapping them all.

Screen Shot 2021-10-18 at 6 50 55 PM

The problem that @andreadelrio found with this approach is that when dragging a control, not having a background could make it look like the controls can be dragged anywhere, when the drop zone is only the top. I am not sure how we can solve this, but I think if we go the route of removing the background in some cases, it should be removed in all cases.

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

LGTM. I like the background-color-while-dragging design that Andrea shared during the sync. Also, +1 to more polish after merging / viewing in the full context of Kibana.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id before after diff
presentationUtil 4 74 +70

History

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

@ThomThomson ThomThomson merged commit 9097344 into elastic:master Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting 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:medium Medium Level of Effort Project:Controls release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Controls] Options List Selection Management
6 participants