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.

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

[Lens] Add color swatch to primary metric dimension #138053

Merged
merged 4 commits into from
Aug 8, 2022
Merged

[Lens] Add color swatch to primary metric dimension #138053

merged 4 commits into from
Aug 8, 2022

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Aug 3, 2022 •
edited by kibanamachine
Loading

Summary

Part of #137769

Screen.Recording.2022-08-03.at.3.43.50.PM.mov

@drewdaemon drewdaemon added release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:prev-minor Backport to the previous minor version (i.e. one version back from main) labels Aug 3, 2022
@drewdaemon drewdaemon mentioned this pull request Aug 3, 2022
Closed
15 tasks
@drewdaemon drewdaemon marked this pull request as ready for review August 4, 2022 12:57
@drewdaemon drewdaemon requested a review from a team as a code owner August 4, 2022 12:57
@elasticmachine
Copy link
Contributor

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

@stratoula
Copy link
Contributor

stratoula commented Aug 8, 2022 •
edited
Loading

The swatch works fine but I found out a behavior that I find confusing.

I have selected a static color (pink). I add a breakdown that will create NA values and I change the mode to dynamic. The NAs are colored pink (the static color). Is it expected?

lens1

As this also happens in main I am approving! Just a question. Why is it necessary to be backported in 8.4? If we want to backport it (dont have any strong objection, I just think this is not a crucial bug but maybe I am wrong) then you have to change the label from fix to skip as this is an unreleased behavior. If you add the label fix, it will be added to our release notes and it might confuse the users :)

stratoula
stratoula approved these changes Aug 8, 2022
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Approving as the goal of this PR is to add a color swatch and this works fine :)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

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.2MB 1.2MB +137.0B

History

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

@flash1293
Copy link
Contributor

About the color override for N/A - I agree, this seems like a bug as there is no way to edit this "fallback" color after switching to dynamic coloring. Could you open a separate issue for this?

@stratoula
Copy link
Contributor

Done! #138241

@drewdaemon
Copy link
Contributor Author

Discussed synchronously as a team and decided it was worth a back port.

@drewdaemon drewdaemon merged commit 5322266 into elastic:main Aug 8, 2022
@kibanamachine kibanamachine mentioned this pull request Aug 8, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 8, 2022
@drewdaemon
[Lens] Add color swatch to primary metric dimension (elastic#138053)
1d231db 
(cherry picked from commit 5322266)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@drewdaemon drewdaemon added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:fix labels Aug 8, 2022
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 8, 2022
@kibanamachine @drewdaemon
[Lens] Add color swatch to primary metric dimension (elastic#138053) (e…
540dce6 
…lastic#138265)

(cherry picked from commit 5322266)

Co-authored-by: Andrew Tate <andrew.tate@elastic.co>
@logeekal logeekal mentioned this pull request Aug 10, 2022
Merged
@drewdaemon drewdaemon mentioned this pull request Aug 11, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stratoula stratoula stratoula approved these changes

@MichaelMarcialis MichaelMarcialis Awaiting requested review from MichaelMarcialis

Assignees
No one assigned
Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.4.0 v8.5.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@drewdaemon @elasticmachine @stratoula @kibana-ci @flash1293 @kibanamachine