[Core] MultiSlider component #2536

jscheiny · 2018-05-22T14:54:42Z

Checklist

Enable CircleCI for your fork
Include tests
Update documentation

Changes proposed in this pull request:

This adds a very general MultiRangeSlider component that ultimately should be able to subsume the Slider and RangeSlider components.

Notes about the component API:

Instead of taking a value/values the component takes a list of handles specified by props. Each handle has:
value - where it gets placed on the slider
type - specifies how the slider handle should be displayed
- "full" - handle appears like it does in a Slider (default)
- "lower" - handle appears like the first slider in a RangeSlider
- "upper" - handle appears like the second slider in a RangeSlider
trackIntent{Above,Below} specifies the color of the track fill above and below (numerically) of the given handle.

Handles are always displayed in ascending numeric order and the values that are returned by onChange and onRelease are always sorted.

In addition, MultiRangeSlider takes a defaultTrackIntent prop which is used in the resolution of how to render the color of the track between two given handles. The way that this is resolved is as follows (and is one of my concerns about the API).

If the left handle has an above intent, use that
If the right handle has a below intent, use that
If the component has a default intent, use that
Otherwise don't color that section

Important: Note that there is a difference between the NONE intent and the absence of an intent entirely (undefined). The former will render a gray track between the two handles and the latter will fall-through to the next color in the hierarchy above.

I previously allowed the specification of colors instead of intents but that didn't seem very blueprint-y.

Reviewers should focus on:

I have explicitly not written tests/docs for this yet, as I would first like to iterate on the API as I'm not convinced what I've written is particularly nice (worked for our use case but don't know if it generalizes well). Once we've iterated and nailed down an API I will gladly write tests and docs for this component.

I've written some questions in the code, but one additional question I have is should this component live in labs first to incubate before moving into core?

Not addressed in this PR (requires follow-up):

To keep this PR simple(r) and easy(er) to review, I have not made many changes to the existing slider components but in 1-2 follow up PRs I would like to:

Refactor Slider and RangeSlider to be simple wrappers around MultiRangeSlider
Remove CoreSlider and refactor into MultiRangeSlider

The second one is questionable, especially if there are plans to add a slider that does not fit into the multi range slider model.

Screenshots

Two tailed slider:

Disabled:

One tailed:

Weird:

Vertical:

In action:

…range-slider

… into review/js/multi-range-slider

blueprint-bot · 2018-05-22T14:57:44Z

Fix lint

Preview: documentation | landing | table

giladgray · 2018-06-01T21:35:26Z

packages/core/src/components/slider/multiSlider.tsx

+     * or between handles for `RangeSlider`.
+     * @default true
+     */
+    showTrackFill?: boolean;


@jscheiny is this prop respected by your impl?

Yes, here: https://github.com/palantir/blueprint/pull/2536/files#diff-b1bce65425b54a1075cd383becfa1663R258

blueprint-bot · 2018-06-01T21:43:26Z

Fix tests

Preview: documentation | landing | table

blueprint-bot · 2018-06-05T00:15:48Z

only one track element, remove some classes

Preview: documentation | landing | table

blueprint-bot · 2018-06-05T17:44:40Z

fix test import

Preview: documentation | landing | table

* write Handle tests for onChange/onRelease movement * Handle maybe calls onChange when released (if different from prop) * rewrite sliderTestUtils: simulateMovement() does it all * enable vertical Handle tests, more utils refactors * refactor Slider & RangeSlider test suites: test handles & track * rewrite MultiSlider tests * tests imports * fix Slider initialValue and displayName

…-range-slider

blueprint-bot · 2018-06-07T21:04:15Z

MultiSlider tests (#2576)

Preview: documentation | landing | table

giladgray · 2018-06-07T21:05:58Z

example is broken. will fix shortly

* maybeRenderAxis => renderLabels * renderFill => renderTracks * border-radius only on track, remove start/end modifiers from segments * fix test utils * add test for handles outside range * fix Label usage in example

blueprint-bot · 2018-06-07T21:18:09Z

MultiSlider refactors (#2569)

Preview: documentation | landing | table

* Slider docs * refactor MultiSliderExample

blueprint-bot · 2018-06-07T22:36:42Z

MultiSlider docs (#2577)

Preview: documentation | landing | table

* utils use ComponentType to support SFCs * drop Slider prefix from Handle types * Handle component extends IHandleProps interface * SliderHandle => MultiSlider.Handle * update tests * fix Handle tests

blueprint-bot · 2018-06-08T00:17:37Z

SliderHandle => MultiSlider.Handle (#2578)

Preview: documentation | landing | table

giladgray

update on what this PR actually does:

replace CoreSlider with MultiSlider component that supports arbitrary handles and track segments
- 👍 CoreSlider was internal, so this isn't actually a breaking change
refactor Slider and RangeSlider to compose MultiSlider internally (instead of extending CoreSlider
- 👍 APIs unchanged for these two components
refactor tests by introducing suite for Handle and greatly simplifying Slider/RangeSlider suites

jscheiny · 2018-06-11T19:21:02Z

packages/core/src/components/slider/handleProps.tsx

+    value: number;
+
+    /** Whether this handle is non-interactive. */
+    disabled?: boolean;


I don't think this prop is ever read.

* MultiSlider fixes * Remove disabled prop * Fix typings

blueprint-bot · 2018-06-12T17:40:33Z

MultiSlider fixes (#2588)

Preview: documentation | landing | table

blueprint-bot · 2018-06-12T18:00:47Z

default prop values in docs

Preview: documentation | landing | table

Jonah Scheinerman and others added 27 commits April 19, 2018 10:48

Multi range slider

f1666d8

Lint and oops

d892981

Use children instead of array of props

e4680a2

Fix lint?

6aeab97

Merge remote-tracking branch 'upstream/develop' into review/js/multi-…

f2c3530

…range-slider

Simplify intent coloring

07d872c

Fix track filling on single slider

c8ac5e9

Fix border radius bug

7d1aa5a

Add todo

13ccec6

Change lower/upper -> start/end and below/above -> before/after

f7e69ae

Fix range slider

b079378

Example component

daccaf8

Fix lint

5977d41

Minor fixes

32208ba

Refactor default props

191425e

Remove import

2f9d274

Address comments

621d193

Use utils function

cb10803

Refactor slider tests

813c082

Merge branch 'develop' into js/multi-range-slider

fbfef2c

Tests

3549691

Merge branch 'js/multi-range-slider' of github.com:jscheiny/blueprint…

fbb187e

… into review/js/multi-range-slider

Fixes for out of order handles

0d12368

Improvements to the example

bbdc8ff

Add default track intent

159c87d

Fix vertical track border radii

8b5f387

Fix lint

0ba20e8

jscheiny mentioned this pull request May 22, 2018

MultiRangeSlider component #2407

Closed

3 tasks

giladgray self-requested a review May 22, 2018 18:12

Fix tests

a23d78c

giladgray reviewed Jun 1, 2018

View reviewed changes

jscheiny added 2 commits June 1, 2018 17:50

Fix example

66c394a

Replace track stops with no interaction handles

5ffe867

giladgray added this to the 3.0.0 milestone Jun 4, 2018

Gilad Gray added 3 commits June 4, 2018 15:34

fix compiler errors, a few refactors

9fa33e8

renames, props, docs

04fc607

only one track element, remove some classes

fa3c176

fix test import

f700b5e

giladgray and others added 2 commits June 7, 2018 13:58

Merge branch 'develop' of github.com:palantir/blueprint into js/multi…

751e193

…-range-slider

MultiSlider refactors (#2569)

6295ae8

* maybeRenderAxis => renderLabels * renderFill => renderTracks * border-radius only on track, remove start/end modifiers from segments * fix test utils * add test for handles outside range * fix Label usage in example

MultiSlider docs (#2577)

ec44d17

* Slider docs * refactor MultiSliderExample

SliderHandle => MultiSlider.Handle (#2578)

1258fcf

* utils use ComponentType to support SFCs * drop Slider prefix from Handle types * Handle component extends IHandleProps interface * SliderHandle => MultiSlider.Handle * update tests * fix Handle tests

giladgray approved these changes Jun 11, 2018

View reviewed changes

jscheiny commented Jun 11, 2018

View reviewed changes

MultiSlider fixes (#2588)

e4681e6

* MultiSlider fixes * Remove disabled prop * Fix typings

default prop values in docs

0b4e7a6

giladgray merged commit 00bbef6 into develop Jun 12, 2018

giladgray deleted the js/multi-range-slider branch June 12, 2018 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] MultiSlider component #2536

[Core] MultiSlider component #2536

jscheiny commented May 22, 2018 •

edited

Loading

blueprint-bot commented May 22, 2018

giladgray Jun 1, 2018

jscheiny Jun 1, 2018

blueprint-bot commented Jun 1, 2018

blueprint-bot commented Jun 5, 2018

blueprint-bot commented Jun 5, 2018

blueprint-bot commented Jun 7, 2018

giladgray commented Jun 7, 2018

blueprint-bot commented Jun 7, 2018

blueprint-bot commented Jun 7, 2018

blueprint-bot commented Jun 8, 2018

giladgray left a comment

jscheiny Jun 11, 2018

blueprint-bot commented Jun 12, 2018

blueprint-bot commented Jun 12, 2018

[Core] MultiSlider component #2536

[Core] MultiSlider component #2536

Conversation

jscheiny commented May 22, 2018 • edited Loading

Checklist

Changes proposed in this pull request:

Reviewers should focus on:

Not addressed in this PR (requires follow-up):

Screenshots

blueprint-bot commented May 22, 2018

Fix lint

giladgray Jun 1, 2018

Choose a reason for hiding this comment

jscheiny Jun 1, 2018

Choose a reason for hiding this comment

blueprint-bot commented Jun 1, 2018

Fix tests

blueprint-bot commented Jun 5, 2018

only one track element, remove some classes

blueprint-bot commented Jun 5, 2018

fix test import

blueprint-bot commented Jun 7, 2018

MultiSlider tests (#2576)

giladgray commented Jun 7, 2018

blueprint-bot commented Jun 7, 2018

MultiSlider refactors (#2569)

blueprint-bot commented Jun 7, 2018

MultiSlider docs (#2577)

blueprint-bot commented Jun 8, 2018

SliderHandle => MultiSlider.Handle (#2578)

giladgray left a comment

Choose a reason for hiding this comment

jscheiny Jun 11, 2018

Choose a reason for hiding this comment

blueprint-bot commented Jun 12, 2018

MultiSlider fixes (#2588)

blueprint-bot commented Jun 12, 2018

default prop values in docs

jscheiny commented May 22, 2018 •

edited

Loading