From 702bcd4f658ede6d53e37cc75130a42dbb75e50e Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 19 Sep 2023 16:35:05 -0400 Subject: [PATCH 1/6] test(range): add failing tests Co-authored-by: Sean Perkins --- .../components/range/test/item/range.e2e.ts | 14 ++ core/src/components/range/test/range.spec.ts | 150 ++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/core/src/components/range/test/item/range.e2e.ts b/core/src/components/range/test/item/range.e2e.ts index 26e17b12e5b..ececb0a2038 100644 --- a/core/src/components/range/test/item/range.e2e.ts +++ b/core/src/components/range/test/item/range.e2e.ts @@ -31,6 +31,20 @@ configs().forEach(({ title, screenshot, config }) => { const list = page.locator('ion-list'); await expect(list).toHaveScreenshot(screenshot(`range-inset-list`)); }); + test('should render adjustments in item', async ({ page }) => { + await page.setContent( + ` + + + + + + `, + config + ); + const list = page.locator('ion-list'); + await expect(list).toHaveScreenshot(screenshot(`range-adjustments`)); + }); }); }); diff --git a/core/src/components/range/test/range.spec.ts b/core/src/components/range/test/range.spec.ts index 2898675c899..79a8752318c 100644 --- a/core/src/components/range/test/range.spec.ts +++ b/core/src/components/range/test/range.spec.ts @@ -1,5 +1,6 @@ import { newSpecPage } from '@stencil/core/testing'; import { Range } from '../range'; +import { Item } from '../../item/item'; let sharedRange; describe('Range', () => { @@ -76,3 +77,152 @@ describe('range id', () => { expect(range.getAttribute('id')).toBe('my-custom-range'); }); }); + +describe.only('range: item adjustments', () => { + it('should add start and end adjustment with no content', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(true); + expect(range.classList.contains('range-item-end-adjustment')).toBe(true); + }); + + it('should add start adjustment with end label and no adornments', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(true); + expect(range.classList.contains('range-item-end-adjustment')).toBe(false); + }); + + it('should add end adjustment with start label and no adornments', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(true); + }); + + it('should add end adjustment with fixed label and no adornments', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(true); + }); + + it('should add start adjustment with floating label', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(true); + expect(range.classList.contains('range-item-end-adjustment')).toBe(true); + }); + + it('should add start adjustment with stacked label', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(true); + expect(range.classList.contains('range-item-end-adjustment')).toBe(true); + }); + + it('should not add adjustment when not in an item', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(false); + }); + + // TODO FW-2997 remove this + it('should not add adjustment with legacy syntax', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(false); + }); + + it('should not add start adjustment when with start adornment', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + +
Start Content
+ + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(false); + }); + + it('should not add end adjustment when with end adornment', async () => { + const page = await newSpecPage({ + components: [Item, Range], + html: ` + +
Start Content
+ + `, + }); + + const range = page.body.querySelector('ion-range'); + expect(range.classList.contains('range-item-start-adjustment')).toBe(false); + expect(range.classList.contains('range-item-end-adjustment')).toBe(false); + }); +}); From fddab4b766ac6c5ff3ab8bc385dd41cca6216eab Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 19 Sep 2023 16:36:56 -0400 Subject: [PATCH 2/6] fix(range): knob is not cut off in item with modern syntax Co-authored-by: Sean Perkins --- core/src/components/range/range.ios.scss | 10 +++++ core/src/components/range/range.ios.vars.scss | 5 +++ core/src/components/range/range.md.scss | 10 +++++ core/src/components/range/range.md.vars.scss | 5 +++ core/src/components/range/range.tsx | 41 +++++++++++++++++-- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/core/src/components/range/range.ios.scss b/core/src/components/range/range.ios.scss index 2b8e5ceb1d8..660df7c1fce 100644 --- a/core/src/components/range/range.ios.scss +++ b/core/src/components/range/range.ios.scss @@ -21,6 +21,16 @@ @include padding($range-ios-padding-vertical, $range-ios-padding-horizontal); } +// TODO FW-2997 update this to $range-ios-padding-horizontal +:host(.range-item-start-adjustment) { + @include padding(null, null, null, $range-ios-padding-horizontal-modern); +} + +// TODO FW-2997 update this to $range-ios-padding-horizontal +:host(.range-item-end-adjustment) { + @include padding(null, $range-ios-padding-horizontal-modern, null, null); +} + :host(.ion-color) .range-bar-active, :host(.ion-color) .range-tick-active { background: current-color(base); diff --git a/core/src/components/range/range.ios.vars.scss b/core/src/components/range/range.ios.vars.scss index 1fba3f3c93e..0dfc374e201 100644 --- a/core/src/components/range/range.ios.vars.scss +++ b/core/src/components/range/range.ios.vars.scss @@ -7,8 +7,13 @@ $range-ios-padding-vertical: 8px !default; /// @prop - Padding start/end of the range +// TODO FW-2997 Update to 24px for modern syntax $range-ios-padding-horizontal: 16px !default; +/// @prop - Padding start/end of the range - modern syntax +// TODO FW-2997 Remove this +$range-ios-padding-horizontal-modern: 24px !default; + /// @prop - Height of the range slider $range-ios-slider-height: 42px !default; diff --git a/core/src/components/range/range.md.scss b/core/src/components/range/range.md.scss index b5b534527a3..492b5232041 100644 --- a/core/src/components/range/range.md.scss +++ b/core/src/components/range/range.md.scss @@ -30,6 +30,16 @@ @include padding($range-md-padding-vertical, $range-md-padding-horizontal); } +// TODO FW-2997 update this to $range-md-padding-horizontal +:host(.range-item-start-adjustment) { + @include padding(null, null, null, $range-md-padding-horizontal-modern); +} + +// TODO FW-2997 update this to $range-md-padding-horizontal +:host(.range-item-end-adjustment) { + @include padding(null, $range-md-padding-horizontal-modern, null, null); +} + :host(.ion-color) .range-bar { background: current-color(base, 0.26); } diff --git a/core/src/components/range/range.md.vars.scss b/core/src/components/range/range.md.vars.scss index ef26283c37c..b12d0639e0a 100644 --- a/core/src/components/range/range.md.vars.scss +++ b/core/src/components/range/range.md.vars.scss @@ -7,8 +7,13 @@ $range-md-padding-vertical: 8px !default; /// @prop - Padding start/end of the range +// TODO FW-2997 Update to 18px for modern syntax $range-md-padding-horizontal: 14px !default; +/// @prop - Padding start/end of the range - modern range +// TODO FW-2997 remove this +$range-md-padding-horizontal-modern: 18px !default; + /// @prop - Height of the range slider $range-md-slider-height: 42px !default; diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 2a6f0e76261..bc51d322a45 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -610,8 +610,41 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop ); } + /** + * Returns true if content was passed to the "start" slot + */ + private get hasStartSlotContent() { + return this.el.querySelector('[slot="start"]') !== null; + } + + /** + * Returns true if content was passed to the "end" slot + */ + private get hasEndSlotContent() { + return this.el.querySelector('[slot="end"]') !== null; + } + private renderRange() { - const { disabled, el, rangeId, pin, pressedKnob, labelPlacement, label } = this; + const { disabled, el, hasLabel, rangeId, pin, pressedKnob, labelPlacement, label } = this; + + const inItem = hostContext('ion-item', el); + + /** + * If there is no start content then the knob at + * the min value will be cut off by the item margin. + */ + const hasStartContent = + (hasLabel && (labelPlacement === 'start' || labelPlacement === 'fixed')) || this.hasStartSlotContent; + + const needsStartAdjustment = inItem && !hasStartContent; + + /** + * If there is no end content then the knob at + * the max value will be cut off by the item margin. + */ + const hasEndContent = (hasLabel && labelPlacement === 'end') || this.hasEndSlotContent; + + const needsEndAdjustment = inItem && !hasEndContent; const mode = getIonMode(this); @@ -624,18 +657,20 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop id={rangeId} class={createColorClasses(this.color, { [mode]: true, - 'in-item': hostContext('ion-item', el), + 'in-item': inItem, 'range-disabled': disabled, 'range-pressed': pressedKnob !== undefined, 'range-has-pin': pin, [`range-label-placement-${labelPlacement}`]: true, + 'range-item-start-adjustment': needsStartAdjustment, + 'range-item-end-adjustment': needsEndAdjustment, })} >