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

Add a (dual) range component #496

Closed
timroes opened this issue Mar 12, 2018 · 14 comments · Fixed by #1485
Closed

Add a (dual) range component #496

timroes opened this issue Mar 12, 2018 · 14 comments · Fixed by #1485
Assignees

Comments

@timroes
Copy link
Contributor

timroes commented Mar 12, 2018

It would be nice to have a range slider in EUI, i.e. a slider with two knobs, that specify a minimum and maximum value. We currently use different implementations across Kibana (Tag Cloud options, Input Control vis), that should rather be implemented via EUI.

@ppisljar
Copy link
Member

screenshot-localhost-5601 2018 04 25 13-51-25

a sample of what tim is talking about (currently used in tag cloud)

or should we replace all such controls with two separate sliders ?

@snide
Copy link
Contributor

snide commented Apr 25, 2018

@cchaos is working on some of this now. Caroline, do you mind posting some screens? We'll get them into components in the next couple weeks.

@cchaos
Copy link
Contributor

cchaos commented Apr 26, 2018

This is the direction we will be going with sliders design-wise.

screen shot 2018-04-26 at 11 10 46 am

@cchaos cchaos changed the title Add range slider component Add a (dual) range component Jul 2, 2018
@cchaos
Copy link
Contributor

cchaos commented Jul 2, 2018

I'm changing this from designer to engineer because the styles are now in there via #932, but a custom component needs to be built to support the dual functionality.

@thomasneirynck
Copy link

thomasneirynck commented Nov 13, 2018

👍

We need something similar in the GIS-app. We need it to select a min/max zoom range, and select min/max size-ranges for data-driven styling.

@thompsongl
Copy link
Contributor

In regards to the API, how much parity will we need with the single-knob version? That is, I assume things like min and max are still needed, as is showInput (probably one on each side, in this case). levels seems questionable based on an initial read, but my understanding of its use cases is lacking.

@cchaos
Copy link
Contributor

cchaos commented Jan 11, 2019

The feeling is that it would look, behave, and have similar props to the range slider that exists now, but with 2 values (min and max) that might also bring with it some validation. Perhaps it's easier to add more options to the current slider to allow for 2 values to be passed back versus trying to duplicate into a new component?

@thompsongl
Copy link
Contributor

It's certainly possible to extend the functionality of EuiRange, but that does use input type=range under the hood. The multiple option was removed from the HTML5 spec, though, so reuse will be pretty limited unless we can use/write a polyfill (e.g., http://leaverou.github.io/multirange/). If not, we may be looking at going away from input entirely for the dual case.

@cchaos
Copy link
Contributor

cchaos commented Jan 11, 2019

I think moving away from the input is just fine. It's better than using a polyfill as those can get out of date quick. Whatever is the easiest really. The GIS team could really use this component before 7.0 (end of January).

@chandlerprall
Copy link
Contributor

If we don't use input I think it makes a lot of to roll a new component for the dual sliders. Target the same API surface with props for min/max changed, but the underlying code should be vastly different.

@thompsongl
Copy link
Contributor

thompsongl commented Jan 22, 2019

Summary of a Zoom with @chandlerprall just now:

  • Not going the polyfill route. Too brittle.
  • It appears we get some good default functionality by using a type=range input (min, max, step, disabled), but we have to do thumb (handle) positioning & click deconflicting ourselves, including hiding the native thumb.
  • Extending the current EuiRange would make for some gnarly if/else logic and file bloat, which makes creating a new component, EuiDualRange (open to naming help), the current direction
  • The above points are all valid even if we bypass input entirely and do it all ourselves with divs , so we'll try with input first. A11y trade-offs will be investigated further

@cchaos
Copy link
Contributor

cchaos commented Jan 22, 2019

Sounds good to me. When you create the docs part can you pull the current EuiRange out of the Form controls page and just create a new one dedicated to both EuiRange and EuiDualRange?

Also, we should consider some DRY techniques that would allow both components to share some internals but leave the specific dual vs singular logic to the individual components.

@thompsongl
Copy link
Contributor

Agreed. And I did see your comment in #1461 related to documentation, so that will be part of the dev.

@thompsongl thompsongl self-assigned this Jan 24, 2019
@thompsongl
Copy link
Contributor

Summary of a Zoom with @cchaos just now:

In terms of an API, the two sliders have the ability to have the same surface, but the space and positioning limitations introduced with the dual case require some design changes.

Tooltips are likely to either overlap with each other or extend beyond the intended container (or browser window). The left/right positioning logic in place doesn't remedy this. Given that the associated text/number inputs are already the recommended method to specify precise values, we decided to forgo tooltips and have implementers rely on the inputs to show exact values.

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

Successfully merging a pull request may close this issue.

7 participants