Skip to content

Commit

Permalink
fix(range): knob is not cut off in item with modern syntax (#28199)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
3 people authored Sep 20, 2023
1 parent 3f06da4 commit 0104d89
Show file tree
Hide file tree
Showing 58 changed files with 234 additions and 3 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 8 additions & 0 deletions core/src/components/range/range.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
@include padding($range-ios-padding-vertical, $range-ios-padding-horizontal);
}

:host(.range-item-start-adjustment) {
@include padding(null, null, null, $range-ios-item-padding-horizontal);
}

:host(.range-item-end-adjustment) {
@include padding(null, $range-ios-item-padding-horizontal, null, null);
}

:host(.ion-color) .range-bar-active,
:host(.ion-color) .range-tick-active {
background: current-color(base);
Expand Down
8 changes: 8 additions & 0 deletions core/src/components/range/range.ios.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@
$range-ios-padding-vertical: 8px !default;

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

/// @prop - Padding start/end of the range - modern syntax
/**
* 24px was chosen so the knob and its
* shadow do not get cut off by the item.
*/
$range-ios-item-padding-horizontal: 24px !default;

/// @prop - Height of the range slider
$range-ios-slider-height: 42px !default;

Expand Down
8 changes: 8 additions & 0 deletions core/src/components/range/range.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
@include padding($range-md-padding-vertical, $range-md-padding-horizontal);
}

:host(.range-item-start-adjustment) {
@include padding(null, null, null, $range-md-item-padding-horizontal);
}

:host(.range-item-end-adjustment) {
@include padding(null, $range-md-item-padding-horizontal, null, null);
}

:host(.ion-color) .range-bar {
background: current-color(base, 0.26);
}
Expand Down
8 changes: 8 additions & 0 deletions core/src/components/range/range.md.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@
$range-md-padding-vertical: 8px !default;

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

/// @prop - Padding start/end of the range - modern range
/**
* 18px was chosen so the knob and its focus/active
* effects do not get cut off by the item.
*/
$range-md-item-padding-horizontal: 18px !default;

/// @prop - Height of the range slider
$range-md-slider-height: 42px !default;

Expand Down
41 changes: 38 additions & 3 deletions core/src/components/range/range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
})}
>
<label class="range-wrapper" id="range-label">
<div
class={{
'label-text-wrapper': true,
'label-text-wrapper-hidden': !this.hasLabel,
'label-text-wrapper-hidden': !hasLabel,
}}
>
{label !== undefined ? <div class="label-text">{label}</div> : <slot name="label"></slot>}
Expand Down
14 changes: 14 additions & 0 deletions core/src/components/range/test/item/range.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
<ion-list inset="true">
<ion-item>
<ion-range value="0" aria-label="true"></ion-range>
</ion-item>
</ion-list>
`,
config
);
const list = page.locator('ion-list');
await expect(list).toHaveScreenshot(screenshot(`range-adjustments`));
});
});
});

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
150 changes: 150 additions & 0 deletions core/src/components/range/test/range.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { newSpecPage } from '@stencil/core/testing';
import { Range } from '../range';
import { Item } from '../../item/item';

let sharedRange;
describe('Range', () => {
Expand Down Expand Up @@ -76,3 +77,152 @@ describe('range id', () => {
expect(range.getAttribute('id')).toBe('my-custom-range');
});
});

describe('range: item adjustments', () => {
it('should add start and end adjustment with no content', async () => {
const page = await newSpecPage({
components: [Item, Range],
html: `
<ion-item>
<ion-range aria-label="Range"></ion-range>
</ion-item>
`,
});

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: `
<ion-item>
<ion-range label="Range" label-placement="end"></ion-range>
</ion-item>
`,
});

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: `
<ion-item>
<ion-range label="Range" label-placement="start"></ion-range>
</ion-item>
`,
});

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: `
<ion-item>
<ion-range label="Range" label-placement="fixed"></ion-range>
</ion-item>
`,
});

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: `
<ion-item>
<ion-range label="Range" label-placement="floating"></ion-range>
</ion-item>
`,
});

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: `
<ion-item>
<ion-range label="Range" label-placement="stacked"></ion-range>
</ion-item>
`,
});

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: `
<ion-range label="Range" label-placement="stacked"></ion-range>
`,
});

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: `
<ion-range legacy="true"></ion-range>
`,
});

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: `
<ion-range label="Range" label-placement="end">
<div slot="start">Start Content</div>
</ion-range>
`,
});

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: `
<ion-range label="Range" label-placement="start">
<div slot="end">Start Content</div>
</ion-range>
`,
});

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);
});
});

0 comments on commit 0104d89

Please sign in to comment.