Skip to content

feat(select): add label slot #27468

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

Merged
merged 13 commits into from
May 15, 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 core/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2698,7 +2698,7 @@ export namespace Components {
*/
"justify": 'start' | 'end' | 'space-between';
/**
* The visible label associated with the select.
* The visible label associated with the select. Use this if you need to render a plaintext label. The `label` property will take priority over the `label` slot if both are used.
*/
"label"?: string;
/**
Expand Down Expand Up @@ -6772,7 +6772,7 @@ declare namespace LocalJSX {
*/
"justify"?: 'start' | 'end' | 'space-between';
/**
* The visible label associated with the select.
* The visible label associated with the select. Use this if you need to render a plaintext label. The `label` property will take priority over the `label` slot if both are used.
*/
"label"?: string;
/**
Expand Down
12 changes: 11 additions & 1 deletion core/src/components/select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -294,14 +294,24 @@ button {
* works on block-level elements. A flex item is
* considered blockified (https://www.w3.org/TR/css-display-3/#blockify).
*/
.label-text {
.label-text,
::slotted([slot="label"]) {
text-overflow: ellipsis;

white-space: nowrap;

overflow: hidden;
}

/**
* If no label text is placed into the slot
* then the element should be hidden otherwise
* there will be additional margins added.
*/
.label-text-wrapper-hidden {
display: none;
}

// Select Native Wrapper
// ----------------------------------------------------------------

Expand Down
61 changes: 52 additions & 9 deletions core/src/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ import type { SelectChangeEventDetail, SelectInterface, SelectCompareFn } from '
/**
* @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use.
*
* @slot label - label - The label text to associate with the select. Use the "labelPlacement" property to control where the label is placed relative to the select. Use this if you need to render a label with custom HTML.
*
* @part placeholder - The text displayed in the select when there is no value.
* @part text - The displayed value of the select.
* @part icon - The select icon container.
*
*/
@Component({
tag: 'ion-select',
Expand Down Expand Up @@ -122,6 +125,10 @@ export class Select implements ComponentInterface {

/**
* The visible label associated with the select.
*
* Use this if you need to render a plaintext label.
*
* The `label` property will take priority over the `label` slot if both are used.
*/
@Prop() label?: string;

Expand Down Expand Up @@ -566,7 +573,7 @@ export class Select implements ComponentInterface {
* TODO FW-3194
* Remove legacyFormController logic.
* Remove label and labelText vars
* Pass `this.label` instead of `labelText`
* Pass `this.labelText` instead of `labelText`
* when setting the header.
*/
let label: HTMLElement | null;
Expand All @@ -576,7 +583,7 @@ export class Select implements ComponentInterface {
label = this.getLabel();
labelText = label ? label.textContent : null;
} else {
labelText = this.label;
labelText = this.labelText;
}

const interfaceOptions = this.interfaceOptions;
Expand Down Expand Up @@ -649,6 +656,30 @@ export class Select implements ComponentInterface {
return Array.from(this.el.querySelectorAll('ion-select-option'));
}

/**
* Returns any plaintext associated with
* the label (either prop or slot).
* Note: This will not return any custom
* HTML. Use the `hasLabel` getter if you
* want to know if any slotted label content
* was passed.
*/
private get labelText() {
const { el, label } = this;

if (label !== undefined) {
return label;
}

const labelSlot = el.querySelector('[slot="label"]');

if (labelSlot !== null) {
return labelSlot.textContent;
Copy link
Contributor

Choose a reason for hiding this comment

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

Passing this as-is to the alert header gives odd results when using a more complicated label structure. For example, opening this:

<ion-select placeholder="Select">
  <div slot="label">
    <div>Select a Fruit</div>
    <ion-note>This will be remembered.</ion-note>
  </div>
  <ion-select-option value="apple">Apple</ion-select-option>
</ion-select>

Leads to this:

odd header

While I can't think of a good way to have Ionic solve this automatically, I noticed you can overwrite it manually using interfaceOptions, and that still works:

<ion-select placeholder="Select" id="weird-label">
  <div slot="label">
    <div>Select a Fruit</div>
    <ion-note>This will be remembered.</ion-note>
  </div>
  <ion-select-option value="apple">Apple</ion-select-option>
</ion-select>

<script>
  const select = document.querySelector('#weird-label');
  select.interfaceOptions = {
    header: 'Select a Fruit'
  };
</script>

Maybe we'll just want to document this pattern and leave it as-is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I can make sure this is documented.

}

return;
}

private getText(): string {
const selectedText = this.selectedText;
if (selectedText != null && selectedText !== '') {
Expand Down Expand Up @@ -696,17 +727,29 @@ export class Select implements ComponentInterface {

private renderLabel() {
const { label } = this;
if (label === undefined) {
return;
}

return (
<div class="label-text-wrapper">
<div class="label-text">{this.label}</div>
<div
class={{
'label-text-wrapper': true,
'label-text-wrapper-hidden': !this.hasLabel,
}}
>
{label === undefined ? <slot name="label"></slot> : <div class="label-text">{label}</div>}
</div>
);
}

/**
* Returns `true` if label content is provided
* either by a prop or a content. If you want
* to get the plaintext value of the label use
* the `labelText` getter instead.
*/
private get hasLabel() {
return this.label !== undefined || this.el.querySelector('[slot="label"]') !== null;
}

/**
* Renders the border container
* when fill="outline".
Expand Down Expand Up @@ -902,10 +945,10 @@ Developers can use the "legacy" property to continue using the legacy form marku
}

private get ariaLabel() {
const { placeholder, label, el, inputId, inheritedAttributes } = this;
const { placeholder, el, inputId, inheritedAttributes } = this;
const displayValue = this.getText();
const { labelText } = getAriaLabel(el, inputId);
const definedLabel = label ?? inheritedAttributes['aria-label'] ?? labelText;
const definedLabel = this.labelText ?? inheritedAttributes['aria-label'] ?? labelText;

/**
* If developer has specified a placeholder
Expand Down
1 change: 1 addition & 0 deletions core/src/components/select/test/a11y/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<main>
<h1>Select - a11y</h1>

<ion-select> <div slot="label">Slotted Label</div> </ion-select><br />
<ion-select label="My Visible Label"></ion-select><br />
<ion-select aria-label="My Aria Label"></ion-select><br />
<ion-select label="My Label" placeholder="Placeholder"></ion-select><br />
Expand Down
59 changes: 57 additions & 2 deletions core/src/components/select/test/label/select.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ configs().forEach(({ title, screenshot, config }) => {

configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('select: label overflow'), () => {
test('label should be truncated with ellipses', async ({ page }) => {
test('label property should be truncated with ellipses', async ({ page }) => {
await page.setContent(
`
<ion-select label="Label Label Label Label Label" placeholder="Select an Item"></ion-select>
Expand All @@ -278,11 +278,24 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, co
const select = page.locator('ion-select');
expect(await select.screenshot()).toMatchSnapshot(screenshot(`select-label-truncate`));
});
test('label slot should be truncated with ellipses', async ({ page }) => {
await page.setContent(
`
<ion-select placeholder="Select an Item">
<div slot="label">Label Label Label Label Label</div>
</ion-select>
`,
config
);

const select = page.locator('ion-select');
expect(await select.screenshot()).toMatchSnapshot(screenshot(`select-label-slot-truncate`));
});
});
});
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('select: alert label'), () => {
test('should use the label to set the default header in an alert', async ({ page }) => {
test('should use the label prop to set the default header in an alert', async ({ page }) => {
await page.setContent(
`
<ion-select label="My Alert" interface="alert">
Expand All @@ -301,5 +314,47 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>

await expect(alert.locator('.alert-title')).toHaveText('My Alert');
});
test('should use the label slot to set the default header in an alert', async ({ page }) => {
await page.setContent(
`
<ion-select interface="alert">
<div slot="label">My Alert</div>
<ion-select-option value="a">A</ion-select-option>
</ion-select>
`,
config
);

const select = page.locator('ion-select');
const alert = page.locator('ion-alert');
const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent');

await select.click();
await ionAlertDidPresent.next();

await expect(alert.locator('.alert-title')).toHaveText('My Alert');
});
test('should use the label prop to set the default header in an alert if both prop and slot are set', async ({
page,
}) => {
await page.setContent(
`
<ion-select label="My Prop Alert" interface="alert">
<div slot="label">My Slot Alert</div>
<ion-select-option value="a">A</ion-select-option>
</ion-select>
`,
config
);

const select = page.locator('ion-select');
const alert = page.locator('ion-alert');
const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent');

await select.click();
await ionAlertDidPresent.next();

await expect(alert.locator('.alert-title')).toHaveText('My Prop Alert');
});
});
});
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.
54 changes: 54 additions & 0 deletions core/src/components/select/test/select.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { newSpecPage } from '@stencil/core/testing';

import { Select } from '../select';

describe('ion-select', () => {
it('should render label prop if only prop provided', async () => {
const page = await newSpecPage({
components: [Select],
html: `
<ion-select label="Label Prop Text"></ion-select>
`,
});

const select = page.body.querySelector('ion-select');

const propEl = select.shadowRoot.querySelector('.label-text');
const slotEl = select.shadowRoot.querySelector('slot[name="label"]');

expect(propEl).not.toBe(null);
expect(slotEl).toBe(null);
});
it('should render label slot if only slot provided', async () => {
const page = await newSpecPage({
components: [Select],
html: `
<ion-select><div slot="label">Label Prop Slot</div></ion-select>
`,
});

const select = page.body.querySelector('ion-select');

const propEl = select.shadowRoot.querySelector('.label-text');
const slotEl = select.shadowRoot.querySelector('slot[name="label"]');

expect(propEl).toBe(null);
expect(slotEl).not.toBe(null);
});
it('should render label prop if both prop and slot provided', async () => {
const page = await newSpecPage({
components: [Select],
html: `
<ion-select label="Label Prop Text"><div slot="label">Label Prop Slot</div></ion-select>
`,
});

const select = page.body.querySelector('ion-select');

const propEl = select.shadowRoot.querySelector('.label-text');
const slotEl = select.shadowRoot.querySelector('slot[name="label"]');

expect(propEl).not.toBe(null);
expect(slotEl).toBe(null);
});
});