[Eui{Dual}Range] Rethink long labels at the boundaries#4781
[Eui{Dual}Range] Rethink long labels at the boundaries#4781thompsongl merged 11 commits intoelastic:masterfrom
Conversation
|
@miukimiu Does this seem reasonable to you from a design perspective? I'll take this out of draft mode if you are ok with the general approach. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
|
@thompsongl I've been testing the PR and there are a few things that I noticed. When we have custom labels, is it possible to make them touch the start/end of the parent container? And the dots should touch the start/end of the track.
Another thing that I noticed is that sometimes the ticks are not created correctly. In the following examples, I was expecting to have a tick at the end of the track: https://gist.github.com/miukimiu/dc60d4f2658ac0a9bbfa695ae9f499de.
|
|
Thanks, @miukimiu! |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
|
Opening this up for review. @miukimiu be aware that there are cases where the dots are not precisely at the end of the track because of percentage math, but those cases are now fewer and the extra space is now smaller. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
|
How are we thinking about the layout of these when no labels exist? It seems there's extra space that we could maybe get rid of in this case?
|
Yep, that's good feedback |
elizabetdev
left a comment
There was a problem hiding this comment.
Thanks @thompsongl. It's looking good. 🎉
I just have a few suggestions/questions.
One the following example the labels are getting out of the container:
<EuiDualRange
fullWidth
min={1619049600000}
max={1619568000000}
step={86400000}
showTicks
value={value2}
onChange={onChange2}
/>
But when we pass custom ticks they get inside of the container:
<EuiDualRange
fullWidth
value={value2}
onChange={onChange2}
min={1619049600000}
max={1619568000000}
showTicks
step={86400000}
ticks={[
{ value: 1619049600000, label: '1619049600000' },
{ value: 1619136000000, label: '1619136000000' },
{ value: 1619222400000, label: '1619222400000' },
{ value: 1619308800000, label: '1619308800000' },
{ value: 1619395200000, label: '1619395200000' },
{ value: 1619481600000, label: '1619481600000' },
{ value: 1619568000000, label: '1619568000000' },
]}
/>
Is this something that can be improved? So that the labels always get inside of the container?
Not if we want to continue to allow text truncation. Label truncation only happens on non-custom labels, the assumption being that if a consumer provides custom labels, they want to see the whole label. The CSS method used to shift custom labels eliminates the ability to truncate text. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
elizabetdev
left a comment
There was a problem hiding this comment.
Thanks @thompsongl,
I tested again in Chrome, Safari, Edge and Firefox and LGTM! 🎉
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4781/ |
Summary
Fixes #4765 by removing dynamic resizing of the range track based on label length, and introduces a new approach to shifting long labels that would otherwise overflow the margins of the components (EuiRange and EuiDualRange).
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Props have proper autodocs and playground toggles