Skip to content

Color palette selection#5443

Merged
rashidkpc merged 30 commits intoelastic:masterfrom
lukasolson:issues/1362
Dec 11, 2015
Merged

Color palette selection#5443
rashidkpc merged 30 commits intoelastic:masterfrom
lukasolson:issues/1362

Conversation

@lukasolson
Copy link
Contributor

This isn't quite finished yet, just because I'm not really happy with the user experience (and it also changes the current experience, which is probably not ideal), but the majority of the work is done.

This PR adds the ability to choose custom colors on visualizations. To pick a new color for a visualization, just click on the corresponding label in the legend, which brings up a list of colors to choose from. Clicking on one of the colors updates the visualization, and if you save the visualization, these changes are persisted.

I don't like the current user experience for a couple of reasons: first of all, because clicking on the legend used to filter by the corresponding value (which is now commented out) and second, because it's just kinda hokey and could use some more polish. (Paging @alt74...)

--EDIT--

Rashid made a bunch of changes to this and now the UI is totally functional and ready to be reviewed.

Closes #1362.

@tomryanx
Copy link

Just a thought: instead of breaking the current behaviour when clicking the legend, add a color wheel logo thing which changes the behaviour of legend clicks. Or have the color wheel bring up the full legend in color selection mode.

Anyway - thanks a lot for bringing us closer to a feature that will be much appreciated!

@rashidkpc rashidkpc self-assigned this Dec 3, 2015
@rashidkpc
Copy link
Contributor

Have a branch working off this, filter buttons still need some work, but the behavior will be that clicking on the legend item brings up the legend item details where you can choose a new color or choose to filter the series in/out. The old behavior only allowed for filtering "in".

screen shot 2015-12-02 at 5 19 30 pm

There is on small gotcha with this system and that is that the colors are mapped to the label, which means if the label changes at all, the color changes. For example, if you change the field formatter for say a numeric field, colors are going to reset. There's no nice way around that right now as the label is used as the series ID. We need to, not in this pull, divorce the view from the model.

@rashidkpc
Copy link
Contributor

Pull to your branch here @lukasolson lukasolson#8

@rashidkpc
Copy link
Contributor

Test should pass with lukasolson#8

@rashidkpc
Copy link
Contributor

screen shot 2015-12-06 at 5 39 29 pm

@lukasolson
Copy link
Contributor Author

When you add multiple metrics, then switch to percentage mode, the colors all change (because the labels have changed). I don't know if this is scoped to this initial issue or not, but it's a funky behavior.

image

image

@rashidkpc
Copy link
Contributor

As noted in the pull to @lukasolson's branch, the above issue is out of scope for this pull. I think we're good functionally here now. Need one more set of eyes on the code

@rashidkpc rashidkpc assigned panda01 and unassigned rashidkpc Dec 11, 2015
@rashidkpc
Copy link
Contributor

We're basically good here. @panda01 noticed an issue when starting up, but we don't think its related to this pull. Once @panda01 gives the LGTM we're good to merge.

@panda01
Copy link
Contributor

panda01 commented Dec 11, 2015

This LGTM!

@brusic brusic mentioned this pull request Dec 11, 2015
rashidkpc added a commit that referenced this pull request Dec 11, 2015
@rashidkpc rashidkpc merged commit e348502 into elastic:master Dec 11, 2015
@rashidkpc rashidkpc mentioned this pull request Dec 11, 2015
@epixa epixa removed the v4.4.0 label Dec 15, 2015
@lukasolson lukasolson deleted the issues/1362 branch February 11, 2016 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants