EuiColorStops #2360

thompsongl · 2019-09-23T13:54:01Z

Summary

Closes #2344 (see full description & background in the issue)

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

ryankeairns · 2019-09-23T15:13:04Z

Thanks for changing the arrow keys, this feels much better!

ryankeairns · 2019-09-25T19:50:51Z

@thompsongl Here is a design PR with minor changes to cursors and new stop styles.

👉 PR => thompsongl#8

I also found a couple of issues:

When typing in the hex input, the delete/backspace key closes the popover, but users will need that for removing and typing new hex values.
Prior to my cursor changes, I notice some flickering that occurs while dragging a stop. It seems as though the cursor gets outside of the stop as you drag. Dragging right, it move outside to the right and vice-versa when dragging left.

Update color stop styles

ryankeairns · 2019-09-25T22:15:29Z

Just checked out latest...

snide

Some small functionality comments. I didn't dig too far into the JS. Functionality seems pretty awesome. ⭐️

src/components/color_picker/color_stops/color_stop_thumb.tsx

src/components/color_picker/color_stops/color_stops.tsx

Cleanup of thumb display

chandlerprall

Didn't find any big issues, very nice work everyone!

src/components/form/range/range_highlight.tsx

src/components/form/range/range_thumb.tsx

src/components/color_picker/color_stops/index.ts

src/components/color_picker/color_stops/color_stops.tsx

src/components/color_picker/color_stops/utils.ts

src/components/color_picker/color_stops/color_stop_thumb.tsx

thompsongl · 2019-09-30T18:09:28Z

Thoughts on moving a stop to the top of the z-index stack on hover?

I'll give this a shot

snide

Tested new additions. They work how I'd expect. From a functional / design / accessibility standpoint this LGTM.

ryankeairns

Looks and works great. Just a couple of docs feedback items (one may be a repeat of Chandler's feedback):

What is the intent of the 'Prop Change' button up top? It wasn't clear to me, but it seems to reset the colors/defaults? If that stays, the wording could be clearer and it may also benefit from looking like a button with a spacer below it.

The last example would benefit from a label - something like 'Custom example'. Going a step further maybe these should be its own stand along section on this page? ( cc:/ @snide )

chandlerprall · 2019-10-01T18:51:15Z

That z-index hover change is, IMO, much better, thank you!

thompsongl · 2019-10-01T18:59:39Z

What is the intent of the 'Prop Change' button up top?

Removing this. It was just there for me to ensure prop changes would propagate.

The last example would benefit from a label

I'm going to rearrange the whole thing, but I did find that the "Display toggles" component doesn't work with form rows

ryankeairns

Yasssss. Great work on the docs cleanup!

nreese · 2019-10-02T01:38:32Z

Is there a way to make min and max optional? In the Kibana Maps use case, we do not know what the range will be.

thompsongl · 2019-10-02T14:15:56Z

Is there a way to make min and max optional?

Currently, no, but we can work out a way where users would be able to define the bounds. Basically, there would always be two stops (one at each end) that will accept any number.

@snide @ryankeairns Is this something we want to add to this PR or do a follow-up?

snide · 2019-10-02T14:41:41Z

@thompsongl since maps is the only known consumer for this one, i think we'd need to add that functionality in before we delivered it. You could always merge now if you just want to make the review smaller for the new functionality.

thompsongl · 2019-10-02T14:48:03Z

I think a smaller, focused review on the new functionality would be better. I can merge this and follow up.

chandlerprall · 2019-10-02T15:04:04Z

Adding a stop at the beginning of the track and then using the keyboard focus + enter creates new stops off the track

thompsongl · 2019-10-02T15:12:56Z

Adding a stop at the beginning of the track and then using the keyboard focus + enter creates new stops off the track

😄 I know why that is. I'll fix it before merging this

chandlerprall

Changes LGTM; pulled and tested locally, only issue found is an edge case in using keyboard navigation to create a new color stop

nreese · 2019-10-02T15:35:40Z

i think we'd need to add that functionality in before we delivered it. You could always merge now if you just want to make the review smaller for the new functionality.

Sounds great to merge as-is and then add a follow up PR to allow setting min and max.

* WIP * WIP: scaling clean up * reverse up/down keys * add left/right movement * update test * doubleClick addStop; clean up; tests removal * use more range components directly * Firefox cleanup * i18n; refactor * WIP: add new stop via click * clean up * better keyboard interaction * add stop styles * empty set; backspace fix; drag handle fix * readOnly and disabled * screen reader * more i18n * cleanup of thumbs * Update src/components/color_picker/color_stops/color_stop_thumb.tsx Co-Authored-By: [email protected] <[email protected]> * thumb title * required label * misc feeback * update zIndex for active thumb * docs refactor * util; basic snapshot tests * color stops tests * thumb snapshot tests * CL * use sorted array for add via keyboard location

thompsongl added 5 commits September 20, 2019 16:58

WIP

1e06162

WIP: scaling clean up

317c77d

reverse up/down keys

68c21ef

add left/right movement

efce438

update test

7dbf922

thompsongl and others added 9 commits September 23, 2019 13:31

doubleClick addStop; clean up; tests removal

6a10f68

use more range components directly

44e74ee

Firefox cleanup

90c338c

i18n; refactor

9719b81

Merge branch 'master' into eui-color-stops

2ce45d0

WIP: add new stop via click

83fc325

clean up

3b77831

better keyboard interaction

006ba4e

add stop styles

d188f7d

thompsongl and others added 2 commits September 25, 2019 15:09

Merge pull request #8 from ryankeairns/eui-color-stops

6907a02

Update color stop styles

empty set; backspace fix; drag handle fix

a1ce7cf

thompsongl added 3 commits September 25, 2019 17:16

readOnly and disabled

f51cf46

screen reader

b61bed9

more i18n

3f0b89d

thompsongl requested review from cchaos, chandlerprall, ryankeairns and snide September 26, 2019 20:50

thompsongl marked this pull request as ready for review September 26, 2019 21:00

snide reviewed Sep 26, 2019

View reviewed changes

src/components/color_picker/color_stops/color_stop_thumb.tsx Outdated Show resolved Hide resolved

src/components/color_picker/color_stops/color_stop_thumb.tsx Outdated Show resolved Hide resolved

src/components/color_picker/color_stops/color_stops.tsx Outdated Show resolved Hide resolved

snide and others added 2 commits September 27, 2019 09:33

cleanup of thumbs

4f8efd5

Merge pull request #9 from snide/design/stop

a57ceff

Cleanup of thumb display

chandlerprall suggested changes Sep 30, 2019

View reviewed changes

thompsongl added 2 commits September 30, 2019 13:54

misc feeback

d078fc8

update zIndex for active thumb

32c7209

snide approved these changes Sep 30, 2019

View reviewed changes

Merge branch 'master' into eui-color-stops

f6c0411

ryankeairns requested changes Oct 1, 2019

View reviewed changes

docs refactor

020a44b

ryankeairns self-requested a review October 1, 2019 19:39

ryankeairns approved these changes Oct 1, 2019

View reviewed changes

thompsongl added 4 commits October 1, 2019 15:14

util; basic snapshot tests

90f6cf4

color stops tests

7259d1a

thumb snapshot tests

5285b90

CL

42ce462

thompsongl requested a review from chandlerprall October 1, 2019 21:48

chandlerprall approved these changes Oct 2, 2019

View reviewed changes

use sorted array for add via keyboard location

6b94aa5

thompsongl merged commit 5b83afb into elastic:master Oct 2, 2019

thompsongl mentioned this pull request Oct 2, 2019

EuiColorStops should allow user to set range #2394

Closed

thompsongl mentioned this pull request Oct 14, 2019

Upgrade EUI to 14.5.0 elastic/kibana#48119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EuiColorStops #2360

EuiColorStops #2360

thompsongl commented Sep 23, 2019 •

edited

Loading

ryankeairns commented Sep 23, 2019

ryankeairns commented Sep 25, 2019 •

edited by thompsongl

Loading

ryankeairns commented Sep 25, 2019

snide left a comment

chandlerprall left a comment

thompsongl commented Sep 30, 2019

snide left a comment

ryankeairns left a comment

chandlerprall commented Oct 1, 2019

thompsongl commented Oct 1, 2019

ryankeairns left a comment

nreese commented Oct 2, 2019

thompsongl commented Oct 2, 2019

snide commented Oct 2, 2019

thompsongl commented Oct 2, 2019

chandlerprall commented Oct 2, 2019

thompsongl commented Oct 2, 2019

chandlerprall left a comment

nreese commented Oct 2, 2019

EuiColorStops #2360

EuiColorStops #2360

Conversation

thompsongl commented Sep 23, 2019 • edited Loading

Summary

Checklist

ryankeairns commented Sep 23, 2019

ryankeairns commented Sep 25, 2019 • edited by thompsongl Loading

ryankeairns commented Sep 25, 2019

snide left a comment

Choose a reason for hiding this comment

chandlerprall left a comment

Choose a reason for hiding this comment

thompsongl commented Sep 30, 2019

snide left a comment

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

chandlerprall commented Oct 1, 2019

thompsongl commented Oct 1, 2019

ryankeairns left a comment

Choose a reason for hiding this comment

nreese commented Oct 2, 2019

thompsongl commented Oct 2, 2019

snide commented Oct 2, 2019

thompsongl commented Oct 2, 2019

chandlerprall commented Oct 2, 2019

thompsongl commented Oct 2, 2019

chandlerprall left a comment

Choose a reason for hiding this comment

nreese commented Oct 2, 2019

thompsongl commented Sep 23, 2019 •

edited

Loading

ryankeairns commented Sep 25, 2019 •

edited by thompsongl

Loading