eui

eui copied to clipboard

Summary

This PR converts EuiRange sliders to Emotion and fixes the following issues:

• [ ] #5923
• [ ] #5725
• [ ] #5186
• [ ] #5228
• [ ] Can we reuse EuiColorStops?

PRs

• #6084

Changelog

WIP

Checklist

• [ ] Checked in both light and dark modes
• [ ] Checked in mobile
• [ ] Checked in Chrome, Safari, Edge, and Firefox
• [ ] Props have proper autodocs and playground toggles
• [ ] Added documentation
• [ ] Checked Code Sandbox works for any docs examples
• [ ] Added or updated jest and cypress tests
• [ ] Checked for breaking changes and labeled appropriately
• [ ] Checked for accessibility including keyboard-only and screenreader modes
• [ ] Updated the Figma library counterpart
• [ ] A changelog entry exists and is marked appropriately

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

@breehall I know this is blocking the release and your Kibana upgrade, so just wanted to give you a quick status update - I'm doing a final pass on the docs page before I merge this in. In particular I'm looking at the playground flyouts - I'm not super happy with their current usability; it feels annoyingly hard to get things in a easy to test state.

Once that's done on either Monday or Tuesday, I was hoping to get your and @1Copenut's eyes on this to QA various input states and help test mouse vs keyboard focus, etc. The more bugs we potentially catch pre-release, the fewer backports we have to do!

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/

👋🏾 Here are a few questions / comments I had after QA-ing:

1. I'm noticing that the Playground is a little buggy. It's probably site wide and not just with this component, but a few things that stood out to me:

• Should I be able to click and drag the range slider from the Playground? This can't be done in the current version either (just as a note).
• Some prop combinations make the range slider disappear from the Playground all together. Some of these include:
  • Increasing step to 3 or above
  • Sometimes activating showTicks - I see that there was a commit to address this (5a10a0c). I think this one might just be funky?

2. It looks like the input that appears when showInput may need to be extended. When isInvalid is true and compressed is false, the numeric value within the input is cut off and can't been seen.

Other than those two things, this looks good. I tested the page with mouse and keyboard controls and keyboard controls successfully as well.

Should I be able to click and drag the range slider from the Playground? This can't be done in the current version either (just as a note).

This is intentional as the onChange is a no-op and is not intended to change the value. This simulated onChange fn is a pattern across multiple playgrounds. Whether or not that's confusing UX is definitely worth discussing but it's definitely something that should be resolved across all playgrounds in a separate concern rather than just this one.

Some prop combinations make the range slider disappear from the Playground all together

Oh interesting. So if you do this on local, you get a very handy error pane that covers the entire screen:

But on staging/prod the message only shows in console logs:

Either way, these are a series of very intentionally thrown errors (that have unit tests written for them) so I'll write it off for now (working the same as production), but I think we should find some way of wrapping an EuiErrorBoundary around the playground or similar.

It looks like the input that appears when showInput may need to be extended. When isInvalid is true and compressed is false, the numeric value within the input is cut off and can't been seen.

Totally agreed! This is already an issue on production it looks like so my preference would be to file an issue + follow up in a separate PR (since changing the width of the input could have unintended effects on the range slider, and @miukimiu may have different ideas on how to resolve the problem).

Thanks for the super thorough QA Bree!!! These are all great items I'd love to see us tackle in the future.

Preview documentation changes for this PR: https://eui.elastic.co/pr_6092/