From 4a99df31dd883abf450cc1792c97512a03626cc4 Mon Sep 17 00:00:00 2001 From: Kyle Taborski Date: Mon, 14 Dec 2020 19:54:08 -0800 Subject: [PATCH 1/7] #1203 slider improving label positioning and width to 192px --- .../@adobe/spectrum-css-temp/components/slider/index.css | 9 +++++++-- packages/@react-spectrum/slider/src/RangeSlider.tsx | 2 +- .../slider/stories/RangeSlider.stories.tsx | 2 +- .../@react-spectrum/slider/stories/Slider.stories.tsx | 4 ++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index 0e80856523a..3accd73ab66 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -66,10 +66,10 @@ governing permissions and limitations under the License. z-index: auto; min-block-size: var(--spectrum-slider-height); - min-inline-size: var(--spectrum-slider-min-width); + min-inline-size: var(--spectrum-global-dimension-size-2400); /* These calculations prevent the track from spilling outside of the control */ - inline-size: calc(100% - calc(var(--spectrum-slider-controls-margin) * 2)); + inline-size: calc(100% - calc(var(--spectrum-slider-controls-margin))); margin-inline-start: var(--spectrum-slider-controls-margin); min-block-size: var(--spectrum-slider-height); @@ -303,6 +303,7 @@ governing permissions and limitations under the License. position: relative; inline-size: auto; + margin-inline-end: calc(var(--spectrum-slider-label-gap-x) / 2 * -1); padding-block-start: var(--spectrum-fieldlabel-padding-top); @@ -323,6 +324,10 @@ governing permissions and limitations under the License. cursor: default; font-feature-settings: "tnum"; text-align: end; + + &:only-child { + text-align: start; + } } .spectrum-Slider-value { diff --git a/packages/@react-spectrum/slider/src/RangeSlider.tsx b/packages/@react-spectrum/slider/src/RangeSlider.tsx index d1ecdf01f04..3f7f93c7e2c 100644 --- a/packages/@react-spectrum/slider/src/RangeSlider.tsx +++ b/packages/@react-spectrum/slider/src/RangeSlider.tsx @@ -60,7 +60,7 @@ function RangeSlider(props: SpectrumRangeSliderProps, ref: FocusableRef + style={{width: `${(1 - state.getThumbPercent(1)) * 100}%`}} /> ); let handles = [0, 1].map(i => ( diff --git a/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx b/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx index abb99583a52..998f0d2f0c8 100644 --- a/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx +++ b/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx @@ -41,7 +41,7 @@ storiesOf('Slider/RangeSlider', module) ) .add( 'label overflow', - () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '100px'}) + () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '200px'}) ) .add( 'showValueLabel: false', diff --git a/packages/@react-spectrum/slider/stories/Slider.stories.tsx b/packages/@react-spectrum/slider/stories/Slider.stories.tsx index 23be159da7d..956cb34555d 100644 --- a/packages/@react-spectrum/slider/stories/Slider.stories.tsx +++ b/packages/@react-spectrum/slider/stories/Slider.stories.tsx @@ -45,11 +45,11 @@ storiesOf('Slider', module) ) .add( 'custom width', - () => render({label: 'Label', width: '200px'}) + () => render({label: 'Label', width: '300px'}) ) .add( 'label overflow', - () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '100px'}) + () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '200px'}) ) .add( 'showValueLabel: false', From 5bb05c4177be0d38b65e876206c10a655c71e9d7 Mon Sep 17 00:00:00 2001 From: Kyle Taborski Date: Mon, 14 Dec 2020 20:32:36 -0800 Subject: [PATCH 2/7] for slider label overflow stories set a width larger then default large min-widht --- packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx | 2 +- packages/@react-spectrum/slider/stories/Slider.stories.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx b/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx index 998f0d2f0c8..93b05945d21 100644 --- a/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx +++ b/packages/@react-spectrum/slider/stories/RangeSlider.stories.tsx @@ -41,7 +41,7 @@ storiesOf('Slider/RangeSlider', module) ) .add( 'label overflow', - () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '200px'}) + () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '300px'}) ) .add( 'showValueLabel: false', diff --git a/packages/@react-spectrum/slider/stories/Slider.stories.tsx b/packages/@react-spectrum/slider/stories/Slider.stories.tsx index 956cb34555d..5bde355ce47 100644 --- a/packages/@react-spectrum/slider/stories/Slider.stories.tsx +++ b/packages/@react-spectrum/slider/stories/Slider.stories.tsx @@ -49,7 +49,7 @@ storiesOf('Slider', module) ) .add( 'label overflow', - () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '200px'}) + () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '300px'}) ) .add( 'showValueLabel: false', From de779d3311e67de17ec8939b44ef31cb14c4c6ab Mon Sep 17 00:00:00 2001 From: Kyle Taborski Date: Wed, 16 Dec 2020 15:57:35 -0800 Subject: [PATCH 3/7] switching from min width to defaul width --- .../spectrum-css-temp/components/slider/index.css | 13 ++++++++++++- .../slider/stories/Slider.stories.tsx | 12 ++++++++++++ .../textfield/stories/Textfield.stories.js | 6 ++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index 3accd73ab66..c5a3cb54581 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -56,6 +56,16 @@ governing permissions and limitations under the License. user-select: none; touch-action: none; + + width: var(--spectrum-global-dimension-size-2400); + + &.spectrum-Slider--label-side { + width: auto; + + /* .spectrum-Slider-controls { + inline-size: calc(var(--spectrum-global-dimension-size-2400) - calc(var(--spectrum-slider-controls-margin))); + }*/ + } } .spectrum-Slider-controls { @@ -66,7 +76,7 @@ governing permissions and limitations under the License. z-index: auto; min-block-size: var(--spectrum-slider-height); - min-inline-size: var(--spectrum-global-dimension-size-2400); + min-inline-size: var(--spectrum-slider-min-width); /* These calculations prevent the track from spilling outside of the control */ inline-size: calc(100% - calc(var(--spectrum-slider-controls-margin))); @@ -325,6 +335,7 @@ governing permissions and limitations under the License. font-feature-settings: "tnum"; text-align: end; + /* When the value is the only child of the label container it's a side label value and we set text-align: start */ &:only-child { text-align: start; } diff --git a/packages/@react-spectrum/slider/stories/Slider.stories.tsx b/packages/@react-spectrum/slider/stories/Slider.stories.tsx index 5bde355ce47..5ba64ee5631 100644 --- a/packages/@react-spectrum/slider/stories/Slider.stories.tsx +++ b/packages/@react-spectrum/slider/stories/Slider.stories.tsx @@ -47,6 +47,10 @@ storiesOf('Slider', module) 'custom width', () => render({label: 'Label', width: '300px'}) ) + .add( + 'custom width small', + () => render({label: 'Label', width: '30px'}) + ) .add( 'label overflow', () => render({label: 'This is a rather long label for this narrow slider element.', maxValue: 1000, width: '300px'}) @@ -75,6 +79,14 @@ storiesOf('Slider', module) 'labelPosition: side', () => render({label: 'Label', labelPosition: 'side'}) ) + .add( + 'labelPosition: side, customWidth', + () => render({label: 'Label', labelPosition: 'side', width: '400px'}) + ) + .add( + 'labelPosition: side, customWidth small', + () => render({label: 'Label', labelPosition: 'side', width: '30px'}) + ) .add( 'min/max', () => render({label: 'Label', minValue: 30, maxValue: 70}) diff --git a/packages/@react-spectrum/textfield/stories/Textfield.stories.js b/packages/@react-spectrum/textfield/stories/Textfield.stories.js index 073e0c1de46..aab95c01588 100644 --- a/packages/@react-spectrum/textfield/stories/Textfield.stories.js +++ b/packages/@react-spectrum/textfield/stories/Textfield.stories.js @@ -109,11 +109,17 @@ storiesOf('TextField', module) .add('custom width', () => render({icon: , validationState: 'invalid', width: '300px'}) ) + .add('custom width small', + () => render({icon: , validationState: 'invalid', width: '30px'}) + ) .add('custom width, quiet', () => render({icon: , validationState: 'invalid', width: '300px', isQuiet: true}) ) .add('custom width, labelPosition: side', () => render({icon: , validationState: 'invalid', width: '500px', labelPosition: 'side'}) + ) + .add('custom width small, labelPosition: side', + () => render({icon: , validationState: 'invalid', width: '30px', labelPosition: 'side'}) ); function render(props = {}) { From 0bc31d8cc7482befdadfab600ce805d4f8cdb852 Mon Sep 17 00:00:00 2001 From: Kyle Taborski Date: Wed, 16 Dec 2020 15:58:38 -0800 Subject: [PATCH 4/7] removing commented out css --- packages/@adobe/spectrum-css-temp/components/slider/index.css | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index c5a3cb54581..a4d54098796 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -61,10 +61,6 @@ governing permissions and limitations under the License. &.spectrum-Slider--label-side { width: auto; - - /* .spectrum-Slider-controls { - inline-size: calc(var(--spectrum-global-dimension-size-2400) - calc(var(--spectrum-slider-controls-margin))); - }*/ } } From 3e9edd36cebc0187875eb05448ffb781cfb8d825 Mon Sep 17 00:00:00 2001 From: Danni Date: Wed, 16 Dec 2020 19:40:07 -0800 Subject: [PATCH 5/7] Use Field css --- .../components/slider/index.css | 31 ++++++++++++++++--- .../@react-spectrum/slider/src/SliderBase.tsx | 3 +- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index a4d54098796..b628f66257d 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -58,9 +58,35 @@ governing permissions and limitations under the License. touch-action: none; width: var(--spectrum-global-dimension-size-2400); + min-inline-size: var(--spectrum-slider-min-width); + + + &.spectrum-Slider--positionTop { + display: inline-flex; + flex-direction: column; + width: var(--spectrum-component-single-line-width); + + .spectrum-Slider-controls { + /* If the user overrides the width of the field, propagate to the inner component */ + width: calc(100% - calc(var(--spectrum-slider-controls-margin)) * 2); + } + } - &.spectrum-Slider--label-side { + /* The side label variant of Field is inline, and fills as much space as needed + * by default. If an explicit width is set, then the field flexes to fill available space. */ + &.spectrum-Slider--positionSide { + display: inline-flex; + align-items: center; width: auto; + + .spectrum-Slider-controls { + flex: 1; + width: var(--spectrum-component-single-line-width); + } + .spectrum-Slider-labelContainer { + margin-inline-end: calc(var(--spectrum-slider-label-gap-x) / 2); + } + } } @@ -72,7 +98,6 @@ governing permissions and limitations under the License. z-index: auto; min-block-size: var(--spectrum-slider-height); - min-inline-size: var(--spectrum-slider-min-width); /* These calculations prevent the track from spilling outside of the control */ inline-size: calc(100% - calc(var(--spectrum-slider-controls-margin))); @@ -309,9 +334,7 @@ governing permissions and limitations under the License. position: relative; inline-size: auto; - margin-inline-end: calc(var(--spectrum-slider-label-gap-x) / 2 * -1); - padding-block-start: var(--spectrum-fieldlabel-padding-top); font-size: var(--spectrum-text-size-text-label); line-height: var(--spectrum-line-height-text-label); diff --git a/packages/@react-spectrum/slider/src/SliderBase.tsx b/packages/@react-spectrum/slider/src/SliderBase.tsx index 8b001c02290..b3aab00274c 100644 --- a/packages/@react-spectrum/slider/src/SliderBase.tsx +++ b/packages/@react-spectrum/slider/src/SliderBase.tsx @@ -128,7 +128,8 @@ function SliderBase(props: SliderBaseProps, ref: FocusableRef) { className={classNames(styles, 'spectrum-Slider', { - 'spectrum-Slider--label-side': labelPosition === 'side', + 'spectrum-Slider--positionTop': labelPosition === 'top', + 'spectrum-Slider--positionSide': labelPosition === 'side', 'is-disabled': isDisabled }, classes, From f85bfe4c1c907f8ca2e48fbfb9e3e5b09c67346d Mon Sep 17 00:00:00 2001 From: Danni Date: Thu, 17 Dec 2020 09:21:29 -0800 Subject: [PATCH 6/7] use positionSide for alignment --- .../spectrum-css-temp/components/slider/index.css | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index b628f66257d..628c0af9ee6 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -79,6 +79,7 @@ governing permissions and limitations under the License. align-items: center; width: auto; + .spectrum-Slider-controls { flex: 1; width: var(--spectrum-component-single-line-width); @@ -87,6 +88,10 @@ governing permissions and limitations under the License. margin-inline-end: calc(var(--spectrum-slider-label-gap-x) / 2); } + &:only-child { + text-align: start; + } + } } @@ -353,11 +358,6 @@ governing permissions and limitations under the License. cursor: default; font-feature-settings: "tnum"; text-align: end; - - /* When the value is the only child of the label container it's a side label value and we set text-align: start */ - &:only-child { - text-align: start; - } } .spectrum-Slider-value { From aee85c9c3b58b734149934f072152e985491075d Mon Sep 17 00:00:00 2001 From: Kyle Taborski Date: Thu, 17 Dec 2020 09:40:27 -0800 Subject: [PATCH 7/7] Update packages/@adobe/spectrum-css-temp/components/slider/index.css This should work now. Co-authored-by: Devon Govett --- packages/@adobe/spectrum-css-temp/components/slider/index.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@adobe/spectrum-css-temp/components/slider/index.css b/packages/@adobe/spectrum-css-temp/components/slider/index.css index 628c0af9ee6..c00df59c627 100644 --- a/packages/@adobe/spectrum-css-temp/components/slider/index.css +++ b/packages/@adobe/spectrum-css-temp/components/slider/index.css @@ -88,7 +88,7 @@ governing permissions and limitations under the License. margin-inline-end: calc(var(--spectrum-slider-label-gap-x) / 2); } - &:only-child { + .spectrum-Slider-value { text-align: start; }