Skip to content

[Unified search] Distinguishes the adhoc from the permanent dataviews on the pickers #143525

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Oct 25, 2022
Merged

[Unified search] Distinguishes the adhoc from the permanent dataviews on the pickers #143525

merged 10 commits into from
Oct 25, 2022

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Oct 18, 2022 •
edited
Loading

Summary

Closes #141806

Adds an icon to indicate that the selected dataview in the picker is an adhoc one and a permanent badge to the dataview list.

image

Also implemented for the lens dataview picker list
image

@stratoula
Copy link
Contributor Author

stratoula commented Oct 18, 2022 •
edited
Loading

Another thing we could also do is to also add the icon on the list (using the prepend property of the EUISelectable) but honestly I don't like it :/

Checked
image

Not checked
image

@MichaelMarcialis @andreadelrio wdyt?

@stratoula stratoula changed the title Adhoc dataviewpicker icon [Unified search] Add an icon to the selected dataview on the picker to indicate that is not persisted Oct 18, 2022
@MichaelMarcialis
Copy link
Contributor

@MichaelMarcialis @andreadelrio wdyt?

I like the idea of indicating temporary data views in the selectable list prior to having users select them, but I agree that the icon doesn't look too great in that prepend slot. Have you tried using the append slot instead? You could also try using an EuiBadge with text identifying it as "Temporary" in that append slot (either with or without the icon). If that's a direction you like, I'd also consider changing the current "Technical preview" EuiBetaBadge to a regular EuiBadge for the sake of keeping things looking consistent with the Temporary badge and focus state enter key indicator.

Example Without Icons

Frame 3

Example With Icons

Frame 2

I personally like the example without icons for the sake of cutting down on the visual clutter in an already robust menu. Let me know if one of those works for you.

stratoula
stratoula commented Oct 19, 2022
View reviewed changes
src/plugins/unified_search/public/dataview_picker/text_languages_list.tsx
cursor: pointer;
`}
/>
<EuiBadge color="hollow">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ We are changing this to EuiBadge for consistency

@stratoula
Copy link
Contributor Author

Thanx @MichaelMarcialis I love your proposal! Done!

@stratoula stratoula added release_note:enhancement backport:skip This commit does not require backporting Feature:Unified search Unified search related tasks v8.6.0 labels Oct 19, 2022
@stratoula stratoula changed the title [Unified search] Add an icon to the selected dataview on the picker to indicate that is not persisted [Unified search] Adds an icon to the selected dataview on the picker to indicate that is not persisted Oct 19, 2022
@stratoula stratoula marked this pull request as ready for review October 19, 2022 08:18
@stratoula stratoula requested review from a team as code owners October 19, 2022 08:18
@stratoula stratoula added Feature:Lens Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) labels Oct 19, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@stratoula stratoula changed the title [Unified search] Adds an icon to the selected dataview on the picker to indicate that is not persisted [Unified search] Distinguishes the adhoc from the permanent dataviews on the pickers Oct 19, 2022
flash1293
flash1293 approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
unifiedSearch 233 234 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.3MB 1.3MB +148.0B
unifiedSearch 260.4KB 260.4KB +21.0B
total +169.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
unifiedSearch 48.7KB 49.6KB +926.0B

History

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

@stratoula stratoula merged commit 1b5ea75 into elastic:main Oct 25, 2022
@kertal kertal mentioned this pull request Nov 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukasolson lukasolson lukasolson approved these changes

@flash1293 flash1293 flash1293 approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Lens Feature:Unified search Unified search related tasks release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Unified Search] Direct ad hoc data views
6 participants
@stratoula @MichaelMarcialis @elasticmachine @kibana-ci @lukasolson @flash1293