Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(range): add label prop #27408

Merged
merged 7 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions angular/src/directives/proxies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1619,14 +1619,14 @@ export declare interface IonRadioGroup extends Components.IonRadioGroup {


@ProxyCmp({
inputs: ['activeBarStart', 'color', 'debounce', 'disabled', 'dualKnobs', 'labelPlacement', 'legacy', 'max', 'min', 'mode', 'name', 'pin', 'pinFormatter', 'snaps', 'step', 'ticks', 'value']
inputs: ['activeBarStart', 'color', 'debounce', 'disabled', 'dualKnobs', 'label', 'labelPlacement', 'legacy', 'max', 'min', 'mode', 'name', 'pin', 'pinFormatter', 'snaps', 'step', 'ticks', 'value']
})
@Component({
selector: 'ion-range',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['activeBarStart', 'color', 'debounce', 'disabled', 'dualKnobs', 'labelPlacement', 'legacy', 'max', 'min', 'mode', 'name', 'pin', 'pinFormatter', 'snaps', 'step', 'ticks', 'value'],
inputs: ['activeBarStart', 'color', 'debounce', 'disabled', 'dualKnobs', 'label', 'labelPlacement', 'legacy', 'max', 'min', 'mode', 'name', 'pin', 'pinFormatter', 'snaps', 'step', 'ticks', 'value'],
})
export class IonRange {
protected el: HTMLElement;
Expand Down
1 change: 1 addition & 0 deletions core/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,7 @@ ion-range,prop,color,"danger" | "dark" | "light" | "medium" | "primary" | "secon
ion-range,prop,debounce,number | undefined,undefined,false,false
ion-range,prop,disabled,boolean,false,false,false
ion-range,prop,dualKnobs,boolean,false,false,false
ion-range,prop,label,string | undefined,undefined,false,false
ion-range,prop,labelPlacement,"end" | "fixed" | "start",'start',false,false
ion-range,prop,legacy,boolean | undefined,undefined,false,false
ion-range,prop,max,number,100,false,false
Expand Down
8 changes: 8 additions & 0 deletions core/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2289,6 +2289,10 @@ export namespace Components {
* Show two knobs.
*/
"dualKnobs": boolean;
/**
* The text to display as the control's label. Use this over the `label` slot if you only need plain text. The `label` property will take priority over the `label` slot if both are used.
*/
"label"?: string;
/**
* Where to place the label relative to the range. `"start"`: The label will appear to the left of the range in LTR and to the right in RTL. `"end"`: The label will appear to the right of the range in LTR and to the left in RTL. `"fixed"`: The label has the same behavior as `"start"` except it also has a fixed width. Long text will be truncated with ellipses ("...").
*/
Expand Down Expand Up @@ -6304,6 +6308,10 @@ declare namespace LocalJSX {
* Show two knobs.
*/
"dualKnobs"?: boolean;
/**
* The text to display as the control's label. Use this over the `label` slot if you only need plain text. The `label` property will take priority over the `label` slot if both are used.
*/
"label"?: string;
/**
* Where to place the label relative to the range. `"start"`: The label will appear to the left of the range in LTR and to the right in RTL. `"end"`: The label will appear to the right of the range in LTR and to the left in RTL. `"fixed"`: The label has the same behavior as `"start"` except it also has a fixed width. Long text will be truncated with ellipses ("...").
*/
Expand Down
3 changes: 1 addition & 2 deletions core/src/components/range/range.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
font-size: $range-md-pin-font-size;
}

// TODO FW-2997 Remove this
::slotted([slot="label"]) {
::slotted([slot="label"]), .label-text {
font-size: initial;
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/components/range/range.scss
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
opacity: 0.3;
}

::slotted([slot="label"]) {
::slotted([slot="label"]), .label-text {
/**
* Label text should not extend
* beyond the bounds of the range.
Expand Down
13 changes: 10 additions & 3 deletions core/src/components/range/range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ export class Range implements ComponentInterface {
*/
@Prop() name = this.rangeId;

/**
* The text to display as the control's label. Use this over the `label` slot if
* you only need plain text. The `label` property will take priority over the
* `label` slot if both are used.
*/
@Prop() label?: string;

/**
* Show two knobs.
*/
Expand Down Expand Up @@ -602,7 +609,7 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
}

private renderRange() {
const { disabled, el, rangeId, pin, pressedKnob, labelPlacement } = this;
const { disabled, el, rangeId, pin, pressedKnob, labelPlacement, label } = this;

const mode = getIonMode(this);

Expand All @@ -629,7 +636,7 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
'label-text-wrapper-hidden': !this.hasLabel,
}}
>
<slot name="label"></slot>
{label !== undefined ? <div class="label-text">{label}</div> : <slot name="label"></slot>}
liamdebeasi marked this conversation as resolved.
Show resolved Hide resolved
liamdebeasi marked this conversation as resolved.
Show resolved Hide resolved
</div>
<div class="native-wrapper">
<slot name="start"></slot>
Expand All @@ -642,7 +649,7 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
}

private get hasLabel() {
return this.el.querySelector('[slot="label"]') !== null;
return this.label !== undefined || this.el.querySelector('[slot="label"]') !== null;
}

private renderRangeSlider() {
Expand Down
14 changes: 9 additions & 5 deletions core/src/components/range/test/a11y/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
<main>
<h1>Range - a11y</h1>

<ion-range><span slot="label">my label</span></ion-range
><br />
<ion-range aria-label="my aria label"></ion-range><br />
<ion-range><span slot="label">my label</span></ion-range>
<br />
<ion-range label="my label"></ion-range>
<br />
<ion-range aria-label="my aria label"></ion-range>
<br />
<ion-range>
<span slot="label">temperature</span>
<ion-icon name="snow" slot="start" aria-hidden="true"></ion-icon>
<ion-icon name="flame" slot="end" aria-hidden="true"></ion-icon> </ion-range
><br />
<ion-icon name="flame" slot="end" aria-hidden="true"></ion-icon>
</ion-range>
<br />
</main>
</body>
</html>
16 changes: 16 additions & 0 deletions core/src/components/range/test/label/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,22 @@ <h2>Placement Fixed</h2>
</div>
</div>

<h1>Label Property</h1>
<div class="grid">
<div class="grid-item">
<h2>Placement Start</h2>
<ion-range label="Temperature"></ion-range>
</div>
<div class="grid-item">
<h2>Placement End</h2>
<ion-range label="Temperature" label-placement="end"></ion-range>
</div>
<div class="grid-item">
<h2>Placement Fixed</h2>
<ion-range label="Temperature" label-placement="fixed"></ion-range>
</div>
</div>

<h1>Slotted Items</h1>
<div class="grid">
<div class="grid-item">
Expand Down
26 changes: 26 additions & 0 deletions core/src/components/range/test/label/range.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,32 @@ configs().forEach(({ title, screenshot, config }) => {
expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-items-fixed`));
});
});

test.describe('range: label prop', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be limited to only one mode and one direction? The label prop doesn't change functionality regardless of mode/direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally left all the configs in because the prop needed some styling separate from the slot, so I figured it would be good to verify no visual regressions in all cases, but I could go either way on it, honestly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the majority is okay as is, then I can be on board.

test('should render label in the start placement', async ({ page }) => {
await page.setContent(`<ion-range label-placement="start" label="Volume"></ion-range>`, config);

const range = page.locator('ion-range');

expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-label-prop-start`));
});

test('should render label in the end placement', async ({ page }) => {
await page.setContent(`<ion-range label-placement="end" label="Volume"></ion-range>`, config);

const range = page.locator('ion-range');

expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-label-prop-end`));
});

test('should render label in the fixed placement', async ({ page }) => {
await page.setContent(`<ion-range label-placement="fixed" label="Volume"></ion-range>`, config);

const range = page.locator('ion-range');

expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-label-prop-fixed`));
});
});
});
});

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.
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.
23 changes: 23 additions & 0 deletions core/src/components/range/test/label/range.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { newSpecPage } from '@stencil/core/testing';

import { Range } from '../../range';

describe('range: label', () => {
it('should prioritize the label prop over the slot', async () => {
const page = await newSpecPage({
components: [Range],
html: `
<ion-range label="Label prop">
<div slot="label">Label slot</div>
</ion-range>
`,
});

const range = page.body.querySelector('ion-range');
const propEl = range?.shadowRoot?.querySelector('.label-text');
const slotEl = range?.shadowRoot?.querySelector('slot[name="label"]');

expect(propEl).not.toBeNull();
expect(slotEl).toBeNull();
});
});
1 change: 1 addition & 0 deletions packages/vue/src/proxies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ export const IonRange = /*@__PURE__*/ defineContainer<JSX.IonRange, JSX.IonRange
'color',
'debounce',
'name',
'label',
'dualKnobs',
'min',
'max',
Expand Down