Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Lens] Color mapping for categorical dimensions #162389
[Lens] Color mapping for categorical dimensions #162389
Changes from 66 commits
93bc658
ff5fdb4
324a54e
b9a83fd
7336f59
c00284c
69f25fc
5ab0768
7d5be20
87c7c97
ae0fc29
733501d
89d336b
79ec603
e5303f3
41d19b0
47a6150
d9b0b35
c9c5b0d
297e6e1
e9bc830
e7f62a9
603139e
fcf0b60
7d4aa40
be1fe7a
701de66
1c1e0ae
2943cec
c9a0f91
3259f0b
ffb01f3
06fb3db
d94aa1b
b87d563
db6c451
5e36f8c
ddd2164
5be7faf
1f8e9f7
23d61c8
04647ee
c123823
2e876ab
0548fc7
04ec3f1
0506999
f54bad2
a7eddf8
738e56a
b7a7b6b
229a50b
6ebd877
385fca4
0e7f29d
3ec9a02
ae2fe91
20d497c
0a5c0cd
a0ac286
d2990b7
15c2fcc
2b889e8
c875cab
422f8c1
2bdc4e8
135f8ea
400fd4d
dda05ce
d8e1b7a
75a704b
60395d4
c7a8036
c491573
cf736ad
f873343
411e6af
71449a3
d442274
15cbef0
63c3168
ded9411
5e92c8b
0475e13
6701a1f
b492532
e662e29
ad7e058
9c5f02f
dba89cc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design review
Root-level configuration flyout
Color configuration flyout
Flyout header
Opt-in switch
Answer: you are right about this, but the idea here was to make it prominent and quickly available for the switch. It could generate confusion, but I believe is fine now for this tech preview. For every new chart this option will be already on ON so only when the hard case of 2 layers with color mapping this could cause confusions. Waiting for @timductive @ninoslavmiskovic opinions.
Palette selector
Change the "Palette" label text to "Color palette".
Change "Palette" selector to use compressed EUI size props.
Scale button group
Change the "Scale" button group to use the
isIconOnly
prop.For the time being, let's use these icons that I created previously for our quick high-fidelity design concept. If anyone has issue with them, Gio and I can work together to iterate on them and get them added formally to EUI.
Relocate the "Scale" button group to be in the same row and to the right of the "Palette" selector, per the wireframes.
Are we confident that the term "Sequential" will resonate more with users than something like "Gradient"? To my mind, "Gradient" seems easier to understand, but I'm curious to know what others think.
Answer gradient is about the color, and sequential is about the scale. I believe in educating the user to the right words even if these don't resonate well because they can then build up a vocabulary that can be reused in other software.
Assignments
General
Please change the "Assignments" label text to "Mapping assignments".
Relocate the "Auto assign categories to colors" switch to be a label append (i.e. right aligned) to the "Assignments" label.
Decrease the "Auto assign categories to colors" label font size to 12px (ex.
<EuiText size="xs">
).Change "Auto assign categories to colors" label text to read "Auto mapping". If you feel strongly that we need any additional description for this feature, consider adding it in an appended
EuiIconTip
.Change the padding for the "Assignments" gray container from 16px to 8px to better match wireframes.
Ensure the spacing between each color assignment row is 8px. Currently, there appears to be no spacing.
The
EuiColorPickerSwatch
components for each row should be 32x32px to match the height of the adjacent compressed form elements. Currently they are 24x24px.The color for the arrow (which is used to indicate whether a color swatch is interactable) appears to always be white. Can we instead dynamically change the color of this arrow (either black or white) depending on the color in the swatch to ensure proper contrast?
The color swatch arrow is set to be 2.5px from the right edge. Can we change to 2px to bring it slightly closer to the right edge and avoid subpixel values (for the sake of those not on retina displays)?
The gap between color swatch and combobox is currently 4px. Please increase to 8px to better match the wireframes.
Drag-and-drop reordering of assignment comboboxes isn't implemented currently. Is this being scoped as a possible future enhancement?
NOTE yes this could be a possible future enhancement
All
EuiComboBox
components within "Assignments" should be utilize EUI's compressed size prop.For lengthy term values, the comboboxes appear to expand horizontally and overflow the assignment container. Can we fix this so the combobox adhears to the width of its container and truncation/line-breaks are utilized as needed?
When the delete assignments buttons are enabled, can we configure it so that they use the
color="text"
styles by default, and then on hover/focus, we change tocolor="danger"
(similar to how we do on layer dimension items)?A term of "Other" currently is available for use in assignments when "Group remaining values as other" is enabled. The original intent was that the "All other terms" catch-all assignment would account for this "Other" term (as well as anything else that hasn't been defined). By that logic, "Other" should never appear as an individual assignment option. Does that make sense? Or do we feel strongly that users can/will want to have a separate color assignment for "Other" versus "All other terms"?
NOTE we want to consider "other" as a category, so that's why it doesn't fall into the "all other terms" bucket
The "All other terms" assignment row should have a 1px border divider separating itself visually from the preceding color assignments, with 8px of space above and below the border.
There should be whitespace to the right of the "All other terms" combobox, so that the right edge of the combobox is correctly aligned with the right edge of all the comboboxes above it.
Color picker popover
Can we adjust the width of the color picker popover so that a maximum 5 swatches fit per row? Doing so will ensure we get a nice, even 5x2 grid of swatches when the maximum of 10 palette colors is in use.
Change the color picker popover's tab interface to be 100% the width of the container so the border tab border extends fully to the popover's bleeding edge.
Change the "Palette" tab text to the a generic term, like "Colors". We are already using the term "Palette" as a section header within that tab, and the duplicative heading may be confusing.
Change the popover's section titles from 14px to 12px (
<EuiTitle size="xxxs">
).Change the "Palette" section title to "Palette colors".
Can we add a full-bleed dividing border between the "Palette" and "Contrast-aware greys" sections?
Change the "Contrast-aware greys" section title to "Neutral colors" and append it with an
EuiIconTip
explaining that "The provided neutral colors are theme-aware and will change appropriately when switching between light and dark themes."In the "Custom" color tab, please remove the buttons to "Use this color" and "Close". Selecting the custom tab should immediately assume the user wishes to use a custom color, and closing popovers already occurs via the escape key or clicking away (like all Kibana popovers).
NOTEdone with debounce
If we've chosen to omit an opacity option for palette colors in the color picker, should we still be offering an opacity option in the "Custom" color tab? I'm thinking we should be consistent and remove opacity from both in that case.
Can we add a secondary hex value input to the "Custom" color tab, similar to how we offer on standard EUI color pickers (
<EuiColorPicker secondaryInputDisplay="bottom">
)?Rather than displaying an ever-present contrast-checker for light/dark themes, I think we should simply show a conditional warning if/when AA contrast-checking doesn't pass for one or both of these themes (i.e. only show when something is wrong). Perhaps we could engineer this in an unobtrusive way by appending a conditional
warning
icon to the aforementioned secondary hex value input (with accompanying tooltip that explains there is an issue with the selected color's contrast in light and/or dark mode). Thoughts?Gradient stops
For the vertical gradient stops interface, let's remove the white arrows being used to indicate each color stop can be interacted with. There is no need for such an indicator here, as the gradient stops will always able to be interacted with (unlike the color swatches). Instead, to make the stops appear more click-friendly, let's follow the design style laid out in EUI's former color stop component by making the following stylistic changes to the color stops:
Change color stop dimenstions to 16x16px.
Add a 3px white border.
Apply a large shadow (
euiShadow(euiThemeContext, 'l')
).Also, perhaps on hover/focus we could add a transition to scale the circle up a bit (happy to help with CSS if needed).
The gradient's track needs a subtle outline or border (in case the chosen color blends in with the page background). Let's use a 1px border of
colors.darkestShade
at 20% opacity to match what EUI uses on their color palette track.Can we have the buttons for adding color stops to the gradient only show only when the user is hovering/focusing the gradient interface (rather then having them be an ever-present element covering the gradient track)? Perhaps we could also add a subtle 0–100% opacity transition as well?
NOTE: I tried but the only thing I can do is to highlight the + button when over the + button, not from the line gradient. We can double check together on how to fix that.
<EuiButtonEmpty color="danger" size="xs">
). Finally, let's add a full-bleed border above it to match the wireframes.Note: I used the term "step" rather then stop because feels more generic
NOTE: fixed now you can remove everything except the last one in whatever direction
NOTE: As already discussed we prefer maintaining the current pattern mechanism until user feedbacks says that is a bad ux
Footer
The space between the "Assignments" gray container and "Add assignment" button should be 4px. Currently it is 8px.
The "Assignments" footer buttons for "Add assignment" and "Invert gradient" should use the
flush="both"
prop to better align to the left edge of the preceding contents. Note that you'll need to add some space between the buttons to compensate for the removal of their padding.Please change the "Invert gradient" icon from
sortAscending
/sortDescending
tosortable
.Potential bugs
NOTE coloring by "date" is not recommended. I can't match partial date written as string to timestamps used in the code also because I don't have a way to distinguish between a date written by the user and different type of category, I can parse only text input. If you need to specify date then we need date pickers on every category that is something too complex for an unused case. Let's collect feedback and see how it works. (also consider the fact that the date histogram could be "auto" computed, meaning that what you describe as date could change depending on the time window. Probably is better to use a color by range instead.
Note Still checking this
NOTE Is a cascade effect actually, the first assigned the only one applied. I've added a warning sign on each duplicate category so the user can decide what to do and which to remove