Skip to content

Commit 0104d89

Browse files
liamdebeasisean-perkinsIonitron
authored
fix(range): knob is not cut off in item with modern syntax (#28199)
Issue number: resolves #27199 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When using the modern range in an item, the knob will get cut off by the item when the value is at either the min or the max. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Range knob is no longer cut off by the item ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> This is an extension of #27188. I decided to make a separate branch/PR since I added tests and changed the implementation a bit. Feel free to take all/some/none of this code. --------- Co-authored-by: Sean Perkins <[email protected]> Co-authored-by: ionitron <[email protected]>
1 parent 3f06da4 commit 0104d89

File tree

58 files changed

+234
-3
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+234
-3
lines changed

core/src/components/range/range.ios.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@
2121
@include padding($range-ios-padding-vertical, $range-ios-padding-horizontal);
2222
}
2323

24+
:host(.range-item-start-adjustment) {
25+
@include padding(null, null, null, $range-ios-item-padding-horizontal);
26+
}
27+
28+
:host(.range-item-end-adjustment) {
29+
@include padding(null, $range-ios-item-padding-horizontal, null, null);
30+
}
31+
2432
:host(.ion-color) .range-bar-active,
2533
:host(.ion-color) .range-tick-active {
2634
background: current-color(base);

core/src/components/range/range.ios.vars.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@
77
$range-ios-padding-vertical: 8px !default;
88

99
/// @prop - Padding start/end of the range
10+
// TODO FW-2997 Remove this
1011
$range-ios-padding-horizontal: 16px !default;
1112

13+
/// @prop - Padding start/end of the range - modern syntax
14+
/**
15+
* 24px was chosen so the knob and its
16+
* shadow do not get cut off by the item.
17+
*/
18+
$range-ios-item-padding-horizontal: 24px !default;
19+
1220
/// @prop - Height of the range slider
1321
$range-ios-slider-height: 42px !default;
1422

core/src/components/range/range.md.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@
3030
@include padding($range-md-padding-vertical, $range-md-padding-horizontal);
3131
}
3232

33+
:host(.range-item-start-adjustment) {
34+
@include padding(null, null, null, $range-md-item-padding-horizontal);
35+
}
36+
37+
:host(.range-item-end-adjustment) {
38+
@include padding(null, $range-md-item-padding-horizontal, null, null);
39+
}
40+
3341
:host(.ion-color) .range-bar {
3442
background: current-color(base, 0.26);
3543
}

core/src/components/range/range.md.vars.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@
77
$range-md-padding-vertical: 8px !default;
88

99
/// @prop - Padding start/end of the range
10+
// TODO FW-2997 Remove this
1011
$range-md-padding-horizontal: 14px !default;
1112

13+
/// @prop - Padding start/end of the range - modern range
14+
/**
15+
* 18px was chosen so the knob and its focus/active
16+
* effects do not get cut off by the item.
17+
*/
18+
$range-md-item-padding-horizontal: 18px !default;
19+
1220
/// @prop - Height of the range slider
1321
$range-md-slider-height: 42px !default;
1422

core/src/components/range/range.tsx

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,8 +610,41 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
610610
);
611611
}
612612

613+
/**
614+
* Returns true if content was passed to the "start" slot
615+
*/
616+
private get hasStartSlotContent() {
617+
return this.el.querySelector('[slot="start"]') !== null;
618+
}
619+
620+
/**
621+
* Returns true if content was passed to the "end" slot
622+
*/
623+
private get hasEndSlotContent() {
624+
return this.el.querySelector('[slot="end"]') !== null;
625+
}
626+
613627
private renderRange() {
614-
const { disabled, el, rangeId, pin, pressedKnob, labelPlacement, label } = this;
628+
const { disabled, el, hasLabel, rangeId, pin, pressedKnob, labelPlacement, label } = this;
629+
630+
const inItem = hostContext('ion-item', el);
631+
632+
/**
633+
* If there is no start content then the knob at
634+
* the min value will be cut off by the item margin.
635+
*/
636+
const hasStartContent =
637+
(hasLabel && (labelPlacement === 'start' || labelPlacement === 'fixed')) || this.hasStartSlotContent;
638+
639+
const needsStartAdjustment = inItem && !hasStartContent;
640+
641+
/**
642+
* If there is no end content then the knob at
643+
* the max value will be cut off by the item margin.
644+
*/
645+
const hasEndContent = (hasLabel && labelPlacement === 'end') || this.hasEndSlotContent;
646+
647+
const needsEndAdjustment = inItem && !hasEndContent;
615648

616649
const mode = getIonMode(this);
617650

@@ -624,18 +657,20 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
624657
id={rangeId}
625658
class={createColorClasses(this.color, {
626659
[mode]: true,
627-
'in-item': hostContext('ion-item', el),
660+
'in-item': inItem,
628661
'range-disabled': disabled,
629662
'range-pressed': pressedKnob !== undefined,
630663
'range-has-pin': pin,
631664
[`range-label-placement-${labelPlacement}`]: true,
665+
'range-item-start-adjustment': needsStartAdjustment,
666+
'range-item-end-adjustment': needsEndAdjustment,
632667
})}
633668
>
634669
<label class="range-wrapper" id="range-label">
635670
<div
636671
class={{
637672
'label-text-wrapper': true,
638-
'label-text-wrapper-hidden': !this.hasLabel,
673+
'label-text-wrapper-hidden': !hasLabel,
639674
}}
640675
>
641676
{label !== undefined ? <div class="label-text">{label}</div> : <slot name="label"></slot>}

core/src/components/range/test/item/range.e2e.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ configs().forEach(({ title, screenshot, config }) => {
3131
const list = page.locator('ion-list');
3232
await expect(list).toHaveScreenshot(screenshot(`range-inset-list`));
3333
});
34+
test('should render adjustments in item', async ({ page }) => {
35+
await page.setContent(
36+
`
37+
<ion-list inset="true">
38+
<ion-item>
39+
<ion-range value="0" aria-label="true"></ion-range>
40+
</ion-item>
41+
</ion-list>
42+
`,
43+
config
44+
);
45+
const list = page.locator('ion-list');
46+
await expect(list).toHaveScreenshot(screenshot(`range-adjustments`));
47+
});
3448
});
3549
});
3650

core/src/components/range/test/range.spec.ts

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { newSpecPage } from '@stencil/core/testing';
22
import { Range } from '../range';
3+
import { Item } from '../../item/item';
34

45
let sharedRange;
56
describe('Range', () => {
@@ -76,3 +77,152 @@ describe('range id', () => {
7677
expect(range.getAttribute('id')).toBe('my-custom-range');
7778
});
7879
});
80+
81+
describe('range: item adjustments', () => {
82+
it('should add start and end adjustment with no content', async () => {
83+
const page = await newSpecPage({
84+
components: [Item, Range],
85+
html: `
86+
<ion-item>
87+
<ion-range aria-label="Range"></ion-range>
88+
</ion-item>
89+
`,
90+
});
91+
92+
const range = page.body.querySelector('ion-range');
93+
expect(range.classList.contains('range-item-start-adjustment')).toBe(true);
94+
expect(range.classList.contains('range-item-end-adjustment')).toBe(true);
95+
});
96+
97+
it('should add start adjustment with end label and no adornments', async () => {
98+
const page = await newSpecPage({
99+
components: [Item, Range],
100+
html: `
101+
<ion-item>
102+
<ion-range label="Range" label-placement="end"></ion-range>
103+
</ion-item>
104+
`,
105+
});
106+
107+
const range = page.body.querySelector('ion-range');
108+
expect(range.classList.contains('range-item-start-adjustment')).toBe(true);
109+
expect(range.classList.contains('range-item-end-adjustment')).toBe(false);
110+
});
111+
112+
it('should add end adjustment with start label and no adornments', async () => {
113+
const page = await newSpecPage({
114+
components: [Item, Range],
115+
html: `
116+
<ion-item>
117+
<ion-range label="Range" label-placement="start"></ion-range>
118+
</ion-item>
119+
`,
120+
});
121+
122+
const range = page.body.querySelector('ion-range');
123+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
124+
expect(range.classList.contains('range-item-end-adjustment')).toBe(true);
125+
});
126+
127+
it('should add end adjustment with fixed label and no adornments', async () => {
128+
const page = await newSpecPage({
129+
components: [Item, Range],
130+
html: `
131+
<ion-item>
132+
<ion-range label="Range" label-placement="fixed"></ion-range>
133+
</ion-item>
134+
`,
135+
});
136+
137+
const range = page.body.querySelector('ion-range');
138+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
139+
expect(range.classList.contains('range-item-end-adjustment')).toBe(true);
140+
});
141+
142+
it('should add start adjustment with floating label', async () => {
143+
const page = await newSpecPage({
144+
components: [Item, Range],
145+
html: `
146+
<ion-item>
147+
<ion-range label="Range" label-placement="floating"></ion-range>
148+
</ion-item>
149+
`,
150+
});
151+
152+
const range = page.body.querySelector('ion-range');
153+
expect(range.classList.contains('range-item-start-adjustment')).toBe(true);
154+
expect(range.classList.contains('range-item-end-adjustment')).toBe(true);
155+
});
156+
157+
it('should add start adjustment with stacked label', async () => {
158+
const page = await newSpecPage({
159+
components: [Item, Range],
160+
html: `
161+
<ion-item>
162+
<ion-range label="Range" label-placement="stacked"></ion-range>
163+
</ion-item>
164+
`,
165+
});
166+
167+
const range = page.body.querySelector('ion-range');
168+
expect(range.classList.contains('range-item-start-adjustment')).toBe(true);
169+
expect(range.classList.contains('range-item-end-adjustment')).toBe(true);
170+
});
171+
172+
it('should not add adjustment when not in an item', async () => {
173+
const page = await newSpecPage({
174+
components: [Item, Range],
175+
html: `
176+
<ion-range label="Range" label-placement="stacked"></ion-range>
177+
`,
178+
});
179+
180+
const range = page.body.querySelector('ion-range');
181+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
182+
expect(range.classList.contains('range-item-end-adjustment')).toBe(false);
183+
});
184+
185+
// TODO FW-2997 remove this
186+
it('should not add adjustment with legacy syntax', async () => {
187+
const page = await newSpecPage({
188+
components: [Item, Range],
189+
html: `
190+
<ion-range legacy="true"></ion-range>
191+
`,
192+
});
193+
194+
const range = page.body.querySelector('ion-range');
195+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
196+
expect(range.classList.contains('range-item-end-adjustment')).toBe(false);
197+
});
198+
199+
it('should not add start adjustment when with start adornment', async () => {
200+
const page = await newSpecPage({
201+
components: [Item, Range],
202+
html: `
203+
<ion-range label="Range" label-placement="end">
204+
<div slot="start">Start Content</div>
205+
</ion-range>
206+
`,
207+
});
208+
209+
const range = page.body.querySelector('ion-range');
210+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
211+
expect(range.classList.contains('range-item-end-adjustment')).toBe(false);
212+
});
213+
214+
it('should not add end adjustment when with end adornment', async () => {
215+
const page = await newSpecPage({
216+
components: [Item, Range],
217+
html: `
218+
<ion-range label="Range" label-placement="start">
219+
<div slot="end">Start Content</div>
220+
</ion-range>
221+
`,
222+
});
223+
224+
const range = page.body.querySelector('ion-range');
225+
expect(range.classList.contains('range-item-start-adjustment')).toBe(false);
226+
expect(range.classList.contains('range-item-end-adjustment')).toBe(false);
227+
});
228+
});

0 commit comments

Comments
 (0)