[Lens] user can change axis color

[drewdaemon]

Summary

Adds the capability for the user to configure the axis colors in an XY visualization.

Resolves #113755

Screen.Recording.2021-12-02.at.3.59.35.PM.mov

Checklist

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

[elasticmachine]

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

[drewdaemon]

I've followed the pattern that we've used for other axis settings such as axisTitlesVisibilitySettings where it is stored in the visualization state in an object containing information about all three axes:

axisTitlesVisibilitySettings:  {
   x: true,
   yLeft: true,
   yRight: true,
},

Then, an expression function lens_xy_axisTitlesVisibilityConfig is defined to turn those three axis arguments into an object to be passed along to the lens_xy_chart function.

Is this the right approach for axis colors? There may be other axis style customization requests in the future (such as axis thickness), and every one of them would require defining a new object on the state and a new expression function. Just thinking out loud.

[flash1293]

It’s not explicitly stated in the issue but in the screenshot the tick labels use the same color which connects them a bit better visually, should we apply the picked color to both? I’m aware in the default case axis and labels have different colors - it depends on the use case. I guess we want to avoid low contrast values which would be difficult. Not sure… Cc @ghudgins @MichaelMarcialis

[MichaelMarcialis]

It’s not explicitly stated in the issue but in the screenshot the tick labels use the same color which connects them a bit better visually, should we apply the picked color to both? I’m aware in the default case axis and labels have different colors - it depends on the use case. I guess we want to avoid low contrast values which would be difficult. Not sure… Cc @ghudgins @MichaelMarcialis

Yeah, this is an interesting challenge. As you already mentioned, I share the same concerns you have about a user's desired axis color adversely affecting the contrast ratio with the background. Off the top of my head, I can think of three possible direction to proceed:

1. Leave as it is currently, with a single color selection being applied to the axis and (and its accompanying ticks for time series), but not applying that color to the tick and axis label text.
2. Apply the user's single color selection to both axis, ticks, and tick/axis labels, but perform a contrast check between label text and background color. If the contrast ratio is too low, Lens automatically adjusts the label text color (using same hue) to bring the ratio high enough to meet accessibility standards.
3. Provide users an option to split the color selection so they can apply colors to axis/ticks and label text separately (assuming they are unhappy with the label text contrast or color sharing).
   • A possible subset of this direction could be to also check the contrast ratio between label text and background color. When too low, warn the user about it so they can make necessary changes.

There may be other options, but these are the ones that came to mind first. Thoughts?

[drewdaemon]

@MichaelMarcialis I personally sorta lean toward something of the flavor of option 3. It retains the value and ease-of-use that I see in option 2 while ultimately keeping the user in control.

But, I don't know our users as well as others. Maybe they wouldn't really care about having that level of configurability. Thoughts anyone (or @ghudgins )?

[ghudgins]

To me, the main way to use this axis color feature is to color both the same. I believe that was @markov00's intent of the screenshot above when he created it (please tell me if otherwise!)

The most control will give us the most possibilities. I could stretch to think of cases where it'd be useful...like picking a lighter shade of the same color for either the labels or the axis for a more nuanced style. however, can we start with coloring both the same and then see if the contrast thing is a real concern? if people want this? There are other warning-style ways of handling overall contrast warnings to the user....and I can pick some shades of white today for my data series (so this problem does exist right now). we can always add color controls later if we find we need to...

[drewdaemon]

So, essentially we are going to allow the color of the axis and the color of the axis label to be different before an axis color is configured. As soon as the user configures one, axis and label will match the selected color.

Edit: do we give them a way to get back to the default state?

[flash1293]

Assuming the recording in the Pr description is up to date - In the screenshot in the issue the picked color is also used for the tick labels (not to be confused with the axis label) - can we adjust this as well?

[markov00]

I'm creating a comment here just to group the conversation under the same thread (is not a comment about that specific line of code):

Quoting the original issue: #113755

When I have multiple axis
And I have a reference line bound to each axis
I need to match the axis to the threshold line color
So I can easily see which threshold belongs to which axis
2nd use case: also improve the integration on canvas where the background color is more likely to be something custom

The described case to me is really specific to a main use case: multiple axis and reference line or, less descriptive case: within Canvas

If we allow the user to freely choose the axis color (and/or the label color) we are achieving the following, unwanted (I hope) situation where:

1. color is a subjective preference, doesn't necessarily bring the same kind of information and joyfulness to different users. A completely free form selection of an axis color doesn't help and can push us in a dead-end, where are no more able to bring back control over the color choices.
2. this doesn't align with our effort of helping the user make the right choices
3. brings us back to the TSVB era, where also the background color was freely selectable by the user and color contrast checks needs to be developed and applied just to that "freedom"
4. this doesn't work out of the box with the dark/light theme mode we currently have: what happens when a user, in light mode, selects black as color and the next user visualizes the same chart in dark mode?

IMHO you should rediscuss this effort and the possible outcomes

[ghudgins]

in light of @markov00's feedback in this PR I'd like to propose this amended design CC @MichaelMarcialis @flash1293 @andrewctate

• Remove the option from the axis menus
• Add the option to reference line configuration flyout as a toggle with a name like "Apply reference line color to axis" (default off)...underneath "Color" seems to make the most sense to me
• Disable this option when there are multiple reference lines on the same axis with a tooltip - "This option is only available when you are displaying a single reference line on this axis"
• Turn off this function (if it was on previously) on adding a 2nd+ reference line in the same axis
• Extra / gold plating: automatically turn this setting on if there are multiple axies with a single reference line on them. I suppose you would do this right as the 2nd reference line is added to the 2nd axis. This is probably out of scope but this is the case we're really trying to solve with this feature

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 9129a1f
• Storybooks Preview

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
lens	243	244	+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	969.7KB	971.0KB	+1.3KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
lens	25	26	+1

Page load bundle

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

id	before	after	diff
lens	40.3KB	41.1KB	+827.0B

Unknown metric groups

API count

id	before	after	diff
lens	260	261	+1

History

• 💔 Build #10817 failed a8f84dd
• 💚 Build #10722 succeeded 33e933e
• 💚 Build #10385 succeeded 67c0f7b
• 💚 Build #10191 succeeded 4e1d60b
• 💔 Build #10132 failed 1ea7e04

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

[flash1293]

Hm, not sure I fully agree. I can also see this setting being useful to disambiguate the data series-axis assignment, not just for reference lines. Also it doesn’t solve the contrast issue.

[ghudgins]

Hm, not sure I fully agree. I can also see this setting being useful to disambiguate the data series-axis assignment, not just for reference lines. Also it doesn’t solve the contrast issue.

Let's have a meeting then so we can discuss (or we can add it to an existing agenda). In addition I'd like to solve contrast warnings in a more general way that is inclusive of all color settings. I think it's actually similar to this warnings-level thing we need for query performance too so I've been adding more notes to #112931

[MichaelMarcialis]

For what it's worth, I'll add my thoughts here regarding @markov00's concerns before our meeting. Forgive my potentially silly question in advance, but is matching a reference line color to an axis color a common use case (or even an occasional edge case)? While I agree that @markov00's original mockup for matching axis color to reference line color looks very aesthetically pleasing, I can’t immediately think of a scenario where I would desire to do that as a user. For me, the more compelling aspect of @markov00's mockup was the treatment of the accompanying text, the color applied to that text's container, and it's position/orientation on the chart. Instead, I imagine the reference line color as something the vast majority of users apply to convey meaning to that line on the chart (i.e. positivity, negativity, etc.).

That said, I'm curious if there’s a better way (such as position, orientation, tooltip, hover effects, etc.) to identify that a reference line is anchored to a particular axis (in a dual axis scenario), rather than hijacking the coloring of that reference line. Even in a dual axis scenario, I'm not convinced that users would want to color their reference lines and axes to match this way, as it strips one layer of meaning away from the reference line (leaving only icon and text as the only alternatives). What's more, as @ghudgins already mentioned above, this concept of matching reference line and axis colors falls apart the moment a given axis has more than one reference line applied. Given that, it feels like the edge of an already edge case.

TL;DR: I'm concerned about adding a lot of technical and UX complexity to an already complex UI for a feature that nobody may end up using.

That said, I do agree with the comments regarding dynamic coloring, in terms of contrast accessibility standards and accounting for both light and dark modes (as I mentioned in some of my earlier comments in this PR). I think regardless of what our ultimate decision is, we should be careful to take this into account.