Skip to content

fix(many): dynamic label support for modern form controls #27156

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.

Sign up for GitHub

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
sean-perkins merged 11 commits into from
Apr 17, 2023
Merged

fix(many): dynamic label support for modern form controls #27156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
26ee079
chore(angular): test apps use modern label syntax
sean-perkins Apr 10, 2023
09153d7
fix(many): dynamic label support for modern form controls
sean-perkins Apr 10, 2023
55ebe1a
fix: use legacy checks, add item label check
sean-perkins Apr 10, 2023
9b5eae4
test(input): modern syntax supports async labels
sean-perkins Apr 10, 2023
60dab25
test(range): add legacy prop to legacy test
sean-perkins Apr 11, 2023
33cf731
test(range): add legacy prop to tests
sean-perkins Apr 11, 2023
9037ae3
test(range): add legacy prop to basic test
sean-perkins Apr 11, 2023
b5b31e5
test(range): add legacy prop to a11y tests
sean-perkins Apr 11, 2023
b514fbc
revert: angular test app searchbar label
sean-perkins Apr 17, 2023
5ea3e5e
chore: whitespace
sean-perkins Apr 17, 2023
a18c2dc
chore: simplify test
sean-perkins Apr 17, 2023
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
23 changes: 23 additions & 0 deletions core/src/components/input/test/item/input.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,27 @@ test.describe('input: item', () => {
const list = page.locator('ion-list');
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/);
});
});
4 changes: 2 additions & 2 deletions core/src/components/range/test/a11y/range.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ test.describe('range: a11y', () => {
await page.setContent(
`<ion-app>
<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-app>
`
Expand Down Expand Up @@ -57,7 +57,7 @@ test.describe('range: a11y', () => {
await page.setContent(
`<ion-app>
<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-app>
`
Expand Down
4 changes: 2 additions & 2 deletions core/src/components/range/test/label/range.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ test.describe('range: label', () => {
test.describe('range: no start or end items', () => {
test('should render a range with no visible label', async ({ page }) => {
await page.setContent(`
<ion-range></ion-range>
<ion-range legacy="true"></ion-range>
`);

const range = page.locator('ion-range');
Expand Down Expand Up @@ -50,7 +50,7 @@ test.describe('range: label', () => {
test.describe('range: start and end items', () => {
test('should render a range with no visible label', async ({ page }) => {
await page.setContent(`
<ion-range>
<ion-range legacy="true">
<ion-icon name="volume-off" slot="start"></ion-icon>
<ion-icon name="volume-high" slot="end"></ion-icon>
</ion-range>
Expand Down
4 changes: 2 additions & 2 deletions core/src/components/range/test/legacy/a11y/range.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ test.describe('range: a11y', () => {
await page.setContent(
`<ion-app>
<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-app>
`
Expand Down Expand Up @@ -41,7 +41,7 @@ test.describe('range: a11y', () => {
await page.setContent(
`<ion-app>
<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-app>
`
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/range/test/legacy/basic/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
<ion-label> Custom pin label </ion-label>
</ion-list-header>
<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-list>
</ion-content>
Expand Down
4 changes: 2 additions & 2 deletions core/src/components/range/test/legacy/basic/range.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test.describe('range: basic', () => {
* TODO FW-2873
*/
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 rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
Expand Down Expand Up @@ -43,7 +43,7 @@ test.describe('range: basic', () => {
});

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 rangeEnd = await page.spyOnEvent('ionKnobMoveEnd');
Expand Down
8 changes: 6 additions & 2 deletions core/src/utils/forms/form-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { findItemLabel } from '@utils/helpers';

type HTMLLegacyFormControlElement = HTMLElement & { label?: string; legacy?: boolean };

/**
Expand All @@ -21,17 +23,19 @@ export const createLegacyFormController = (el: HTMLLegacyFormControlElement): Le
* can check to see if the component has slotted text
* in the light DOM.
*/
const hasLabelProp = (controlEl as any).label !== undefined || hasLabelSlot(controlEl);
const hasLabelProp = controlEl.label !== undefined || hasLabelSlot(controlEl);
const hasAriaLabelAttribute =
controlEl.hasAttribute('aria-label') ||
// Shadow DOM form controls cannot use aria-labelledby
(controlEl.hasAttribute('aria-labelledby') && controlEl.shadowRoot === null);
const legacyItemLabel = findItemLabel(controlEl);

/**
* Developers can manually opt-out of the modern form markup
* by setting `legacy="true"` on components.
*/
legacyControl = controlEl.legacy === true || (!hasLabelProp && !hasAriaLabelAttribute);
legacyControl =
controlEl.legacy === true || (!hasLabelProp && !hasAriaLabelAttribute && legacyItemLabel !== null);
}
return legacyControl;
};
Expand Down