Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 13.3k
feat(range): add label prop#27408
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
Uh oh!
There was an error while loading. Please reload this page.
feat(range): add label prop #27408
Changes from all commits
c419810e2d4b39321d3b474aa4d3c63f6d1a8a2aad39d03f4File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. | ||
| */ | ||
| @@ -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); | ||
| @@ -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.
Uh oh!There was an error while loading. Please reload this page. | ||
| </div> | ||
| <div class="native-wrapper"> | ||
| <slot name="start"></slot> | ||
| @@ -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() { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -125,6 +125,32 @@ configs().forEach(({ title, screenshot, config }) => { | ||
| expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-items-fixed`)); | ||
| }); | ||
| }); | ||
| test.describe('range: label prop', () => { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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`)); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| 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(); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.