Skip to content

[EuiRange sliders Emotion feature] Merge amsterdam and legacy styles#6084

Merged
elizabetdev merged 1 commit intoelastic:feature/emotion-range-slidersfrom
elizabetdev:merge-range-sliders-amsterdam-and-legacy-styles
Jul 29, 2022
Merged

[EuiRange sliders Emotion feature] Merge amsterdam and legacy styles#6084
elizabetdev merged 1 commit intoelastic:feature/emotion-range-slidersfrom
elizabetdev:merge-range-sliders-amsterdam-and-legacy-styles

Conversation

@elizabetdev
Copy link
Copy Markdown
Contributor

Summary

This PR is the initial step for the EuiRange sliders Emotion conversion. This is going to be merged into a feature branch and the idea is to then have separate PRs to make it easy the review.

On this first PR, I'm basically just moving all the range sliders Amsterdam styles and merging them with the *.scss from the component folder. I took this decision because the range sliders had a lot of Amsterdam styles. This way I will have all the styles in just one place to start the Emotion conversion.

I'm skipping the changelog because I see this as a pre Emotion step.

How to review?

All the range sliders should look the same.

Checklist

  • Checked in both light and dark modes
  • Checked in Chrome, Safari, Edge, and Firefox

@elizabetdev elizabetdev added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Jul 28, 2022
@kibanamachine
Copy link
Copy Markdown

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

@elizabetdev elizabetdev marked this pull request as ready for review July 28, 2022 10:44
@elizabetdev elizabetdev requested a review from cchaos July 29, 2022 12:38
Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice! I did a quick overlay visual comparison and they exactly the same. I just scanned the code since most of it will change anyway.

One piece of advice moving forward is to also look into the color range picker and see if we can combine some things like having the levels actually just be the color stops.

@elizabetdev
Copy link
Copy Markdown
Contributor Author

One piece of advice moving forward is to also look into the color range picker and see if we can combine some things like having the levels actually just be the color stops.

Thanks, @cchaos. I'll look into it. 👍🏽

@elizabetdev elizabetdev merged commit 9ad05b7 into elastic:feature/emotion-range-sliders Jul 29, 2022
cee-chen added a commit that referenced this pull request Dec 6, 2022
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants