[Range sliders] Implement new levels design#6265
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
@thompsongl I have the levels and ranges working. I'll probably adjust the colors and contrast at the end. But now I need some help implementing the colors for the thumbs. The idea is when levels exist, the thumb should get the level color as in the following design. Can you help me implement this feature? |
I'll look today! |
|
Haven't looked at tests/snapshots for c56998f yet so I'll take another look at this tomorrow. Feel free to make adjustments before then! |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Thanks for the help @thompsongl. I found one small issue. On click, the thumb gets the color overridden: But it's just a small CSS fix and I can do it. 👍🏽 |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
I'm opening the PR for review. @chandlerprall, we've talked about having a If you pick a very light color on a light background consumers can see right away that is not going to work. For example: So we have to assume that colors are going to be picked wisely. Also, this I searched for the usage of range sliders with levels in Kiabana and Cloud and I just found one custom colors usage. Which is ok. The others usages is using our color palettes which works well. |
| const handleRef = (node: HTMLDivElement | null) => { | ||
| setTrackWidth(node?.clientWidth ?? 0); | ||
| }; |
There was a problem hiding this comment.
[optional microperf] useCallback
| const handleRef = (node: HTMLDivElement | null) => { | |
| setTrackWidth(node?.clientWidth ?? 0); | |
| }; | |
| const handleRef = useCallback((node: HTMLDivElement | null) => { | |
| setTrackWidth(node?.clientWidth ?? 0); | |
| }, []); |
There was a problem hiding this comment.
When I useCallback , in the following example the highlight doesn't get the right width:
There was a problem hiding this comment.
ahh super interesting. I was wondering how the width updates were responding to changes 😅 it's not technically the most correct use of React but I'm hoping to avoid detecting clientWidth via percentages in any case - will investigate!
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Dangit, I totally forgot this was a branch against a feature branch and merged main into it. Hang on a sec, rebasing... |
…olors to new file - nb: separate file is required for webpack to not throw a fit over circular dependencies
+ remove extra semicolons, since the utils already contain ending semicolons
428fb4c to
673420d
Compare
cee-chen
left a comment
There was a problem hiding this comment.
QA is looking good to me!
As a heads up, while reviewing I found a few things to clean up that in the feature branch/outside of the scope of this PR - was hoping to open up a follow-up PR with more range cleanups and assign that to you after this if that's cool :)
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6265/ |
|
Thanks, @constancecchen for all the cleanup
Sounds good 👍🏽 |
…ments (#6092) * Merged range sliders amsterdam and legacy styles (#6084) * [Emotion] Convert range sliders (#6116) * Adding all necessary emotion style files * WIP styles * Converted range_label * Converted range mixins * Converted dual_rangee * Converted range_draggable * Added range_highlight * Converted range_thumb * Converted range_tooltip * Removed EuiRangeTooltip compressed prop * Moved euiFormControlSize mixin to form.styles.ts * Added euiFormVariables * Converted range_slider * Updating snapshots with 2 tests failing * sasslint fix * mark min and max as having default values * selector fix * compressed types fix * snapshot updates * Fixing range track top position * Revert ticks example * Fix logical css * Removing unnecessary styles * Remove trailing slash from import statements, update dtsgenerator script to remove trailing slashes from imports * Converting px to variables and adding `shouldRenderCustomStyles` * Adding custom thumbs so that we can have z-index and put them above levels * Adding aria label on custom thumb * use default input thumb * Improving comments. Some styles fixes. * Adressing PR review Co-authored-by: Greg Thompson <thompson.glowe@gmail.com> Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * [Range sliders] Implement new levels design (#6265) * Adding liner gradient on top of levels * Styling highlight with levels * Fix level range not showing for single ranges * thumb colors match levels * update snapshot * Fix focus issue * Fixed levels position when no ticks are available * Update src/components/form/range/range_levels.styles.ts Co-authored-by: Constance <constancecchen@users.noreply.github.com> * Adressing PR review * Converting some range highlight inline styles to Emotion * Getting levels colors in thumb as `currentcolor` * [misc] DRY out color array * [cleanup] DRY out various consts, types, & utils around range level colors to new file - nb: separate file is required for webpack to not throw a fit over circular dependencies * [more cleanup] move new level color util to new file * [misc linting] remove unnecessary newlints from single-line strings + remove extra semicolons, since the utils already contain ending semicolons Co-authored-by: Greg Thompson <thompson.glowe@gmail.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com> Co-authored-by: Constance Chen <constance.chen.3@gmail.com> * [Range sliders docs] Add non linear intervals example (#6330) * [Range sliders docs] Add non linear example * Changing examples source types to TSX * [PR feedback] Remove typescript need for any - using `event.currentTarget` over `target` solves most of the single range issues + refactor single instances of callbacks to be inline instead of instantiating a var Co-authored-by: Constance Chen <constance.chen.3@gmail.com> * Adding CL * [Feature] Range sliders QA fixes (#6432) * Fix tests and `fullWidth` * Simplified `calc()` to `mathWithUnits` * Wrapped atransition in `euiCanAnimate` * Replaced `<Fragment/>` with `</>` * Removed unnecessary class names * Fixing but with `repeating-linear-gradient` in Safari * Encoded SVGs again to avoid masks than could lead to possible IDs conflicts * `fullWidth` PR feedback - remove need for modifier class + update snapshots - prefer not to pass `fullWidth` as an arg into param - let there be a 2nd max-width property override instead * Explain why style is being skipped * [PR feedback] Remove need to pass `disabled` to inner style fn + fix typo on className * Remove unnecessary `classNames()` around static classes * Remove `.euiRangeLevels--` modifier classes - existing Kibana selectors can use `[class*="euiRangeLevels-color"]` instead * Remove unnecessary `.euiRangeTooltip__value` modifier class if values can only be left or right, there's no point to targeting both - just target the main `__` class * Fix overflow-x/y on non-FF browsers * Clean up pseudo tick CSS - remove unnecessary border CSS (not sure what it's doing?) and add clarifying CSS comment - simplify various Emotion conditionals to ternaries * Convert various inline style positioning to logical properties * Clean up unnecessary `calc`s in favor of `mathWithUnits` * Remove unused `::-ms-track` browser CSS - We no longer support IE and Edge does not appear to need this styling * DRY out various static `px` width - most notable one: we're calling small `2px`-based border widths all over the place, which should get its own `thumbBorderWidth` var + reorder several properties - prefer display/flow properties first, then color/appearance * DRY out EuiRangeTooltip CSS - remove `transition` - `box-shadow` doesn't even exist on the tooltip and `transform` is not meaningfully changing - remove both double pseudo element selectors - there isn't a need for 2 elements to represent 1 arrow + combine just `::before` selector - DRY out various CSS being applied to both left and right CSS - if it's being applied to both, it can just go on the base CSS - tweak positioning of tooltip arrow - 1px offset does not appear to be needed any longer - DRY out use of `euiFontSize` + reorder several properties - prefer display/flow properties first, then color/appearance * EuiRangeTicks - fixes & cleanup - Fix `selected` styling never actually correctly applying (already occurring in prod) + remove duplicate/unused border-radius property + remove duplicate `::before` pseudo selector CSS - it's already being set in `hasPseudoTickMark` via `tickStyles` * Restore thumb focus color to prod appearance - the black focus color does not match the primary color we use for the rest of the thumb/track * Fix tick marks not being correctly centered on the thumb - 2px is what's on prod, not 4px - i think this is supposed to line up with the border width? * Revert SVG stripes change + make stripe size dynamic to match custom border width changes * Actually fix stripe gradient issue for Safari see https://css-tricks.com/stripes-css/#aa-funky-town - using percentages and then `background-size` appears to solve the issue * Clean up draggable CSS - other focus selectors are not having any visible difference whatsoever in latest FF/Edge/Safari * Massively clean up focus styles - now that we no longer have to support IE and can use `:focus-visible` on its own, there's a whole lot of cruft we can remove, including JS `hasFocus` tracking (vs CSS built-in focus) - Move highlight range to last DOM item- some CSS used in place of the JS `hasFocus` requires the `~` sibling selector - make the focus color a variable so that it can be changed in one place if needed + clean up reset styles - half of them aren't needed (tested in FF+Edge+Safari) * Remove functionally unused `euiRangeTrackSize` fns - if it's not being used in more than one place, it doesn't really need to be a fn/isn't DRYing anything out * Remove unused group CSS - it's not doing anything that I can tell with prepend/append * Misc CSS cleanup - remove not-especially-helpful comments - remove unnecessary syntax - remove unnecessary !important - DRY out logical properties usage applicable Co-authored-by: Constance Chen <constance.chen.3@gmail.com> * Changelog update to note removed Sass vars - most vars/mixins were left intact due to `EuiColorStop` usage - they should be removed after either EuiColorStop is migrated or all EuiForm components are migrated * [Perf/tech debt] Convert EuiRangeTrack to function component + optimize via useMemo + address TODO via useEffect + switch to RTL over Enzyme - Enzyme can't process the `useEffect` behavior in render/shallow * [Perf/tech debt] Memoize EuiRangeLevels + split out logic into internal only sub-component with its own memoization logic + split out validation fn to separate const to match other files + switch to RTL over Enzyme - Enzyme can't process the `useEffect` behavior in render/shallow * [Perf] Memoize objs etc. + FC instances of `logicalStyles()` where possible * DRY out repeated `trackWidth` ref logic to just a single passed prop - useCallback doesn't work on refs / causes them to stop working, so let's improve perf by not watching/setting state on 3 diff nodes within EuiRange - make `EuiRangeTrack` in charge of the single ref and pass the value down as an optional render fn - I strongly recommend turning off whitespace changes when viewing this diff * Fix/clean up passing of `compressed` prop - Fix `EuiRangeWrapper` to actually change height on `compressed` flag - `EuiRangeLevels`, `EuiRangeHighlight`, `EuiRangeSlider`, and `EuiRangeDraggable` do nothing with `compressed` and don't need to be passed the prop - in the case of `EuiRangeTicks`, compressed is used but is already defined in the ...rest spread and initial types def and doesn't need to be re-defined * SQUASH WITH Memoize EuiRangeLevels * [Docs][Types] DRY out many repeated types + more thoroughly document prop behavior - We're repeating a lot of the same types over and over and if our types ever change for whatever reason it's going to be annoying to update 5+ files - More importantly, we're also (incorrectly) repeating type/prop docs between both EuiRange and EuiDualRange and this way / putting them in one place makes it much easier to compare and contrast docs - NOTE: Kibana imports of `EuiRangeTicks` should now import directly from `@elastic/eui` and not reach into the component + DRY out EuiDualRange tests & remove `requiredProps` where it's not relevant ot the test * [EuiDualRange] Require `min` and `max` props - it's not clear to me why `min`/`max` isn't required on EuiDualRange when it's documented as such & has the same default behavior as EuiRange + clean up tests * [Docs] Improve playground experience for EuiRange & EuiDualRange - currently there's a whole bunch of errors thrown if you just try to do something like toggle `showTicks` - this should resolve that frustrating experience - the types cleanup in a previous commit should also have resolved many unnecessary/incorrect props showing up in the playground * [PR feedback] Fix compressed input thumb position; document `compressed` visual changes in changelog Co-authored-by: Greg Thompson <thompson.glowe@gmail.com> Co-authored-by: Chandler Prall <chandler.prall@gmail.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com> Co-authored-by: Constance Chen <constance.chen.3@gmail.com>






Summary
This PR implements the new design for levels in EuiRange and EuiDualRange..
The PR goes to the feature branch #6092.
Design
Checklist