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.

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

EuiColorStops with user-defined ranges #2396

Merged
merged 15 commits into from
Oct 16, 2019

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Oct 2, 2019

Summary

Fixes #2394 by removing min and max as required props and providing default (but changeable) values. The range extents are purely for visual reference and can be set/reset via user interaction.

Checklist

- [ ] Checked in dark mode
- [ ] Checked in mobile

  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests

- [ ] Checked for breaking changes and labeled appropriately

  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@nreese
Copy link
Contributor

nreese commented Oct 2, 2019

Are the thumbs at the missing range extent needed? Could you just default to a min/max and then if a user sets the value to a value greater than the max then that becomes the new max?

@thompsongl
Copy link
Contributor Author

@nreese I've updated it to be closer to what you've suggested. It now sets min/max defaults if not given, and user-added stops will reset those values. There's still work to be done in resetting min/max after they've been set by users, but I want to be sure that this is close to what you had in mind.

@snide
Copy link
Contributor

snide commented Oct 3, 2019

Gave this a brief check. I think the premise and feedback feel right. Don't forget docs.

@thompsongl thompsongl changed the title [WIP] EuiColorStops with user-defined ranges EuiColorStops with user-defined ranges Oct 3, 2019
@nreese
Copy link
Contributor

nreese commented Oct 3, 2019

It is a little jarring when starting with an empty ramp and the first click in the slider creates a stop to the far left. Instead, could the stop stay were the user clicked in the slider? With only a single stop, the marker should be more near the center instead to the far left.

Is it possible to provide some buffer to on the ends of the min and max stops (like 5% of the length)? In the case of maps, anything to the left of the min stop is not given a style and will be colored grey. Anything to the right of max stop will be given the color of the max stop. Either way, data can and will be outside of the min/max stops. Showing how data reacts before the first stop and after the last stop would be really useful for users.

@thompsongl
Copy link
Contributor Author

With only a single stop, the marker should be more near the center instead to the far left

We could work something out here. I think the main adjustment would be to remove the current behavior of always using added stops as the bounds. The problem likely lies, then, in discerning the default bounds. That is, the current default range for and empty set is 0-100. If I add a stop in the middle, and change its value to 200, should 200 then become the max, or should the max become 400 so the added stop stays in the middle? Maybe the fact that the user explicitly changes the value is the deciding factor.

Showing how data reacts before the first stop and after the last stop would be really useful for users

This is a good idea. @ryankeairns, if we were to do this, do you have any thoughts on how to indicate that this "extra" space is not interactive? Also, should the color of this out-of-bounds area be editable by the user? Any other thoughts?

@thompsongl
Copy link
Contributor Author

thompsongl commented Oct 3, 2019

All that ^ to say, we're trying to visually represent an adjustable, near-infinite scale in finite space. So I appreciate the thought and feedback, @nreese

@nreese
Copy link
Contributor

nreese commented Oct 3, 2019

do you have any thoughts on how to indicate that this "extra" space is not interactive? Also, should the color of this out-of-bounds area be editable by the user? Any other thoughts?

The outer margins should be intractable. If the user creates a new stop past the previous max or min, then the new stop becomes the new max or min and the size of the bar adjusts accordingly.

@ryankeairns
Copy link
Contributor

Hey, just getting back after being away. I've read through this and am thinking through some possible design solutions. I'm in training during afternoons this week, but will try and get back with some ideas.

@thompsongl
Copy link
Contributor Author

I'm going to move forward with the basics of @nreese's feedback and we can sync designs when you get ideas together

@thompsongl
Copy link
Contributor Author

Most recent changes:

  • Area "below" first stop is gray, which is more correct than before, indicating the true start value
  • Buffer on each end of the range where an explicit min/max is not given
    • This buffer is 5% of the scale right now
    • It is still interactive
    • Note the buffer on the max end is not gray, this is because the idea is that the last stop extends infinitely or to the max value
  • When no min/max is given, we work with the default min/max for as long as possible
    • Until a stop exceeds one end the range
    • There may still be jumps/shifting when adding/moving stops outside of the range, but that is due to necessary adjustments to the scale

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 8, 2019

Good progress!

The description of the low end behavior, and the gray color below, seems intuitive. I'm curious how it should work when the min is 0 and you try to go lower... @nreese can there be a negative min? (fyi, it breaks if you attempt that now).

The max stop and right buffer was less intuitive. I understand the coloring and that you'd want to enter a new max beyond that, but not being able to drag to the end, as is stands, felt to me like something had gone wrong from an interaction design standpoint.

Perhaps we should use some kind of visual bar or 'partially disabled' stop to help clarify that situation. It could still be interactive but maybe you can only change the stop value? As opposed to clicking in that last section and having it move your new stop back to the max. Alternatively, it may be that the current interaction becomes clearer in combination with the bar? Let's talk it through 🤔

Some initial sketches along that last point:

Screenshot 2019-10-08 16 06 18

@thompsongl
Copy link
Contributor Author

how it should work when the min is 0 and you try to go lower...(fyi, it breaks if you attempt that now)

Ah that's just a bug. I can get that fixed. If the min prop has actually been set to 0, though, it won't let a user enter a negative value.

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 8, 2019

One other concept that goes a slightly different direction would be to display actual min and max text inputs. It's more UI, but it may be worth the clarity here. In this case, the buffer and square points would not be interactive and, thus, the min max would not be color stops but instead be replaced by inputs along with the bar markers.

Visually, I think this route does a good job of signifying what exactly is happening. The min/max inputs align with the bars indicating that anything below or above would be colored as indicated.

Screenshot 2019-10-08 16 15 22

@nreese
Copy link
Contributor

nreese commented Oct 9, 2019

@nreese can there be a negative min

The range could be negative. The color stops need to match to user's data and that data can be negative

@thompsongl
Copy link
Contributor Author

After working through these latest code changes, I've realized something that I think @nreese has been getting at, which is that for a lot of cases min and max don't matter at all. It's very possible that a user doesn't want to cap the data set, but instead give a "from here to infinity" value for the last thumb. With that in mind, adding more formality to the min/max concept feels off.
I agree that the interaction design isn't quite right, though, when dragging the last thumb to the right and its value is changing but you can never reach the end.

@thompsongl
Copy link
Contributor Author

thompsongl commented Oct 9, 2019

Another thought is that it seems likely that users will have stop values in mind before beginning. So I'd imagine that entering known values via the text input comprises a good deal of the interaction cases, which is a case that is supported well right now.
I wonder if this is a component where we need some user feedback to debunk our assumptions

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 9, 2019

It never hurts to get that feedback. Also, by plain definition, there is something inherently off with going beyond a max. As you're hinting at, maybe there just isn't a max in those cases. I'll keep thinking on this and once we have some options, let's run these examples by some internal folks.

🤔 some input or switch that accepts a value or none (i.e. max: none)

@snide
Copy link
Contributor

snide commented Oct 9, 2019

@bcamper You have any opinions here? Sounds like we need some direction from the maps team.

@nreese
Copy link
Contributor

nreese commented Oct 9, 2019

there is something inherently off with going beyond a max. As you're hinting at, maybe there just isn't a max in those cases

The component should handle use cases where there is no max. The last stop just colors anything beyond the stop value. I know a max is needed to size the component but the concept of a max does not really exist for color ramps

@bcamper
Copy link

bcamper commented Oct 9, 2019

Agree with @nreese. Logically, there's a distinction between how values are mapped to colors, and what range of data you're showing. It's common map-wise to set the color min/max to somewhere within the total value range, e.g. if you want to only emphasize features that are above a baseline value, you might set the color palette "min" value to the median or average, so that all the values that are average and below are just lumped into one color (this requires some kind of insight into the data like a histogram, which we're also looking at).

In terms of this component, I think that's what the "ends" of the UI are signifying, and if there's a way to more clearly communicate that "values above/below this point will have this last color", that would be great.

Here's an example from Esri where the stops and histogram are integrated, and the min/max is capped to a more narrow range in the middle of the data (so outliers don't wash out the important values). Because it's all integrated (not something I think we can/want to pursue immediately), the color ramp can communicate those "caps"/color continuation directly:

esri - value range slider color

There's also some interesting prior color stops UI art in places like Datawrapper: https://academy.datawrapper.de/article/132-how-to-use-the-color-palette-tool

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 10, 2019

There are a couple of changes I would like to try:

  1. Instead of displaying an unreachable segment at the ends of the bar, could we add the 5% whenever a new stop value is entered that exceeds either the current min or max? This way you can drag the stop throughout the entire bar while still indicating that any values beyond your left or rightmost color stop will take on the color of the bar beyond that point. In other words, only add the extra bit when a new min or max (i.e. a value beyond the current min/max) is entered. I realize we may not want to start with a min/max, per se, but technically speaking we are setting these values (currently it is 0 to 100).
  2. Related, we may want to offer a prop to restrict the range to the supplied min/max that essentially disables the behavior described in (1). In this case, a validation error would be displayed if a user enters a value outside of the min/max defined in the component props. I don't have a good sense of how necessary this case is, so maybe it's enough to go with scenario (1) to start.

One additional thought for those implementing this component in their app... it may improve the experience if the min/max are calculated on the fly. In other words, say a user selects a field, a check is then done to find the min max value and, perhaps, round down/up from those to set the starting end points.

This way if you have a data set ranging from 107 to 10,798, you start with 100 and 11,000 (for example) instead of 0 and 100. It's not absolutely necessary given you can enter any stop value upon adding a new stop (as described above), but it would reduce the need for users to discover how to expand the default range of 1 to 100. If we agree this is a good recommendation, it could be something we write into the EUI docs.


@thompsongl I think these may be reversed in the current example:

Screenshot 2019-10-10 09 03 37

@thompsongl
Copy link
Contributor Author

I think these may be reversed in the current example

They're correct given the existing concepts.
min is 0 in the first one, so you can't go below 0, but you can move the right thumb to infinity because no max is specified.
In the second one, the right thumb can't go beyond 100 because max is 100, but you can move the left thumb to negative infinity.

@thompsongl
Copy link
Contributor Author

Results of a Zoom chat with @snide and @nreese:

  • As is, this component is at feature parity with the current repeating input implementation currently in Maps (users pick stops and colors at will, with no bounds)
  • As such, we can move forward with the current features and interactions, and get feedback from devs and users.
  • We'll handle additions and changes iteratively and more Maps-specific features (re: links above) will probably be built by the Maps team.
  • Likely future additions to this component will include number labels above/below each thumb for better scannability.
  • I'll attempt to add ability for newly added stops to open their popovers automatically on creation. (to be done in this PR)

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Checked functionality in browser 💯 . Small note on some sass, otherwise LGTM.

src/components/color_picker/color_stops/_color_stops.scss Outdated Show resolved Hide resolved
@thompsongl thompsongl merged commit dbb9e08 into elastic:master Oct 16, 2019
@ryankeairns
Copy link
Contributor

I'm all for getting this out there and collecting feedback as there are several open questions that would benefit from it, but I also feel that a little more time could have been spent on the min/max interaction (see item 1. in my previous comment). Having part of the slider be unreachable (via drag) is not only confusing, but it also leads to some other odd results as seen below:

max_slide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EuiColorStops should allow user to set range
5 participants