mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-07 06:57:02 +08:00
fix(many): dynamic label support for modern form controls (#27156)
<!-- Please refer to our contributing documentation for any questions on submitting a pull request, or let us know here if you need any help: https://ionicframework.com/docs/building/contributing --> <!-- Some docs updates need to be made in the `ionic-docs` repo, in a separate PR. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation for details. --> <!-- 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. --> Developers that are using Ionic v7 are experiencing an issue where implementations that are intended to use the modern control syntax will render with the legacy syntax and a warning will be displayed. The issue is most easily reproduced by not assigning a label to the control and then asynchronously assigning a label after a duration. Angular example: ```html <ion-item> <ion-input [label]="label"></ion-input> </ion-item> ``` ```ts @Component({ ... }) export class MyComponent { @Input() label?: string; // initially unset ngOnInit() { setTimeout(() => this.label = 'Hello world', 500); } } ``` <!-- Issues are required for both bug fixes and features. --> Issue URL: resolves #27085 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Form controls that do not have a decorative label or `aria-label`/`aria-labelledby` assigned, will default render as modern controls. - Legacy form implementations that render an `<ion-label>` within the item, will continue to render with the legacy template and a warning will be displayed in the console. - Modern form syntax supports dynamically set labels ## 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. --> Legacy implementations that do not have a decorative label and do not specify `aria-label` on the control, will be upgraded to the modern syntax. For example: ```html <ion-item> <ion-input></ion-input> </ion-item> ``` Developers that do not want to update to the modern syntax yet should add the `legacy="true"` attribute to their form control. ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev-build: `7.0.2-dev.11681157690.1060bc7f` When migrating the range tests to modern syntax, I observed a visual clipping issue. This is being addressed in: https://github.com/ionic-team/ionic-framework/pull/27188. This PR simply adds the legacy flag so that screenshots are the same as `main`.
This commit is contained in:
@ -36,4 +36,27 @@ test.describe('input: item', () => {
|
|||||||
const list = page.locator('ion-list');
|
const list = page.locator('ion-list');
|
||||||
expect(await list.screenshot()).toMatchSnapshot(`input-inset-list-no-fill-${page.getSnapshotSettings()}.png`);
|
expect(await list.screenshot()).toMatchSnapshot(`input-inset-list-no-fill-${page.getSnapshotSettings()}.png`);
|
||||||
});
|
});
|
||||||
|
test('input renders as modern when label is set asynchronously', async ({ page }, testInfo) => {
|
||||||
|
testInfo.annotations.push({
|
||||||
|
type: 'issue',
|
||||||
|
description: 'https://github.com/ionic-team/ionic-framework/issues/27085',
|
||||||
|
});
|
||||||
|
|
||||||
|
await page.setContent(
|
||||||
|
`<ion-item>
|
||||||
|
<ion-input></ion-input>
|
||||||
|
</ion-item>
|
||||||
|
`
|
||||||
|
);
|
||||||
|
const input = page.locator('ion-input');
|
||||||
|
// Initial template should be modern
|
||||||
|
await expect(input).not.toHaveClass(/legacy-input/);
|
||||||
|
|
||||||
|
// Update the input label
|
||||||
|
await input.evaluate((el: HTMLIonInputElement) => (el.label = 'New label'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Template should still be modern
|
||||||
|
await expect(input).not.toHaveClass(/legacy-input/);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -25,7 +25,7 @@ test.describe('range: a11y', () => {
|
|||||||
await page.setContent(
|
await page.setContent(
|
||||||
`<ion-app>
|
`<ion-app>
|
||||||
<ion-content>
|
<ion-content>
|
||||||
<ion-range min="0" max="100" value="80"></ion-range>
|
<ion-range min="0" max="100" value="80" legacy="true"></ion-range>
|
||||||
</ion-content>
|
</ion-content>
|
||||||
</ion-app>
|
</ion-app>
|
||||||
`
|
`
|
||||||
@ -57,7 +57,7 @@ test.describe('range: a11y', () => {
|
|||||||
await page.setContent(
|
await page.setContent(
|
||||||
`<ion-app>
|
`<ion-app>
|
||||||
<ion-content>
|
<ion-content>
|
||||||
<ion-range min="0" max="100" value="50" pin="true"></ion-range>
|
<ion-range min="0" max="100" value="50" pin="true" legacy="true"></ion-range>
|
||||||
</ion-content>
|
</ion-content>
|
||||||
</ion-app>
|
</ion-app>
|
||||||
`
|
`
|
||||||
|
|||||||
@ -5,7 +5,7 @@ test.describe('range: label', () => {
|
|||||||
test.describe('range: no start or end items', () => {
|
test.describe('range: no start or end items', () => {
|
||||||
test('should render a range with no visible label', async ({ page }) => {
|
test('should render a range with no visible label', async ({ page }) => {
|
||||||
await page.setContent(`
|
await page.setContent(`
|
||||||
<ion-range></ion-range>
|
<ion-range legacy="true"></ion-range>
|
||||||
`);
|
`);
|
||||||
|
|
||||||
const range = page.locator('ion-range');
|
const range = page.locator('ion-range');
|
||||||
@ -50,7 +50,7 @@ test.describe('range: label', () => {
|
|||||||
test.describe('range: start and end items', () => {
|
test.describe('range: start and end items', () => {
|
||||||
test('should render a range with no visible label', async ({ page }) => {
|
test('should render a range with no visible label', async ({ page }) => {
|
||||||
await page.setContent(`
|
await page.setContent(`
|
||||||
<ion-range>
|
<ion-range legacy="true">
|
||||||
<ion-icon name="volume-off" slot="start"></ion-icon>
|
<ion-icon name="volume-off" slot="start"></ion-icon>
|
||||||
<ion-icon name="volume-high" slot="end"></ion-icon>
|
<ion-icon name="volume-high" slot="end"></ion-icon>
|
||||||
</ion-range>
|
</ion-range>
|
||||||
|
|||||||
@ -9,7 +9,7 @@ test.describe('range: a11y', () => {
|
|||||||
await page.setContent(
|
await page.setContent(
|
||||||
`<ion-app>
|
`<ion-app>
|
||||||
<ion-content>
|
<ion-content>
|
||||||
<ion-range min="0" max="100" value="80"></ion-range>
|
<ion-range min="0" max="100" value="80" legacy="true"></ion-range>
|
||||||
</ion-content>
|
</ion-content>
|
||||||
</ion-app>
|
</ion-app>
|
||||||
`
|
`
|
||||||
@ -41,7 +41,7 @@ test.describe('range: a11y', () => {
|
|||||||
await page.setContent(
|
await page.setContent(
|
||||||
`<ion-app>
|
`<ion-app>
|
||||||
<ion-content>
|
<ion-content>
|
||||||
<ion-range min="0" max="100" value="50" pin="true"></ion-range>
|
<ion-range min="0" max="100" value="50" pin="true" legacy="true"></ion-range>
|
||||||
</ion-content>
|
</ion-content>
|
||||||
</ion-app>
|
</ion-app>
|
||||||
`
|
`
|
||||||
|
|||||||
@ -249,7 +249,7 @@
|
|||||||
<ion-label> Custom pin label </ion-label>
|
<ion-label> Custom pin label </ion-label>
|
||||||
</ion-list-header>
|
</ion-list-header>
|
||||||
<ion-item>
|
<ion-item>
|
||||||
<ion-range pin min="1" step="0.1" max="2" id="customLabel"></ion-range>
|
<ion-range pin min="1" step="0.1" max="2" id="customLabel" legacy="true"></ion-range>
|
||||||
</ion-item>
|
</ion-item>
|
||||||
</ion-list>
|
</ion-list>
|
||||||
</ion-content>
|
</ion-content>
|
||||||
|
|||||||
@ -15,7 +15,7 @@ test.describe('range: basic', () => {
|
|||||||
* TODO FW-2873
|
* TODO FW-2873
|
||||||
*/
|
*/
|
||||||
test.fixme('should emit start/end events', async ({ page }, testInfo) => {
|
test.fixme('should emit start/end events', async ({ page }, testInfo) => {
|
||||||
await page.setContent(`<ion-range value="20"></ion-range>`);
|
await page.setContent(`<ion-range value="20" legacy="true"></ion-range>`);
|
||||||
|
|
||||||
const rangeStart = await page.spyOnEvent('ionKnobMoveStart');
|
const rangeStart = await page.spyOnEvent('ionKnobMoveStart');
|
||||||
const rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
|
const rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
|
||||||
@ -43,7 +43,7 @@ test.describe('range: basic', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('should emit start/end events, keyboard', async ({ page }) => {
|
test('should emit start/end events, keyboard', async ({ page }) => {
|
||||||
await page.setContent(`<ion-range value="20"></ion-range>`);
|
await page.setContent(`<ion-range value="20" legacy="true"></ion-range>`);
|
||||||
|
|
||||||
const rangeStart = await page.spyOnEvent('ionKnobMoveStart');
|
const rangeStart = await page.spyOnEvent('ionKnobMoveStart');
|
||||||
const rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
|
const rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
|
||||||
|
|||||||
@ -1,3 +1,5 @@
|
|||||||
|
import { findItemLabel } from '@utils/helpers';
|
||||||
|
|
||||||
type HTMLLegacyFormControlElement = HTMLElement & { label?: string; legacy?: boolean };
|
type HTMLLegacyFormControlElement = HTMLElement & { label?: string; legacy?: boolean };
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -21,17 +23,19 @@ export const createLegacyFormController = (el: HTMLLegacyFormControlElement): Le
|
|||||||
* can check to see if the component has slotted text
|
* can check to see if the component has slotted text
|
||||||
* in the light DOM.
|
* in the light DOM.
|
||||||
*/
|
*/
|
||||||
const hasLabelProp = (controlEl as any).label !== undefined || hasLabelSlot(controlEl);
|
const hasLabelProp = controlEl.label !== undefined || hasLabelSlot(controlEl);
|
||||||
const hasAriaLabelAttribute =
|
const hasAriaLabelAttribute =
|
||||||
controlEl.hasAttribute('aria-label') ||
|
controlEl.hasAttribute('aria-label') ||
|
||||||
// Shadow DOM form controls cannot use aria-labelledby
|
// Shadow DOM form controls cannot use aria-labelledby
|
||||||
(controlEl.hasAttribute('aria-labelledby') && controlEl.shadowRoot === null);
|
(controlEl.hasAttribute('aria-labelledby') && controlEl.shadowRoot === null);
|
||||||
|
const legacyItemLabel = findItemLabel(controlEl);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Developers can manually opt-out of the modern form markup
|
* Developers can manually opt-out of the modern form markup
|
||||||
* by setting `legacy="true"` on components.
|
* by setting `legacy="true"` on components.
|
||||||
*/
|
*/
|
||||||
legacyControl = controlEl.legacy === true || (!hasLabelProp && !hasAriaLabelAttribute);
|
legacyControl =
|
||||||
|
controlEl.legacy === true || (!hasLabelProp && !hasAriaLabelAttribute && legacyItemLabel !== null);
|
||||||
}
|
}
|
||||||
return legacyControl;
|
return legacyControl;
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user