fix(overlays): do not overwrite id set in htmlAttributes (#29722)

Issue number: resolves #29712

---------

<!-- 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?
In every type of overlay, the auto incremented overlay id is overwriting
any id set in htmlAttributes.

## What is the new behavior?
The id in htmlAttributes now takes precedence. 

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

---------

Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
This commit is contained in:
Mikel Hamer
2024-07-24 10:17:56 -04:00
committed by GitHub
parent 05913c3cc3
commit 92ce563c40
21 changed files with 401 additions and 305 deletions

View File

@ -310,7 +310,9 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
}
componentWillLoad() {
setOverlayId(this.el);
if (!this.htmlAttributes?.id) {
setOverlayId(this.el);
}
}
componentDidLoad() {

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { ActionSheet } from '../action-sheet';
it('action sheet should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [ActionSheet],
html: `<ion-action-sheet is-open="true"></ion-action-sheet>`,
});
let actionSheet: HTMLIonActionSheetElement;
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet).not.toBe(null);
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-1');
// Remove the action sheet from the DOM
actionSheet.remove();
await page.waitForChanges();
// Create a new action sheet to verify the id is incremented
actionSheet = document.createElement('ion-action-sheet');
actionSheet.isOpen = true;
page.body.appendChild(actionSheet);
await page.waitForChanges();
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same action sheet again should reuse the existing id
actionSheet.isOpen = false;
await page.waitForChanges();
actionSheet.isOpen = true;
await page.waitForChanges();
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { ActionSheet } from '../action-sheet';
import { h } from '@stencil/core';
describe('action-sheet: id', () => {
it('action sheet should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [ActionSheet],
html: `<ion-action-sheet is-open="true"></ion-action-sheet>`,
});
let actionSheet: HTMLIonActionSheetElement;
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet).not.toBe(null);
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-1');
// Remove the action sheet from the DOM
actionSheet.remove();
await page.waitForChanges();
// Create a new action sheet to verify the id is incremented
actionSheet = document.createElement('ion-action-sheet');
actionSheet.isOpen = true;
page.body.appendChild(actionSheet);
await page.waitForChanges();
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same action sheet again should reuse the existing id
actionSheet.isOpen = false;
await page.waitForChanges();
actionSheet.isOpen = true;
await page.waitForChanges();
actionSheet = page.body.querySelector('ion-action-sheet')!;
expect(actionSheet.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [ActionSheet],
template: () => <ion-action-sheet htmlAttributes={{ id }} overlayIndex={-1}></ion-action-sheet>,
});
const alert = page.body.querySelector('ion-action-sheet')!;
expect(alert.id).toBe(id);
});
});

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Alert } from '../alert';
it('alert should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Alert],
html: `<ion-alert is-open="true"></ion-alert>`,
});
let alert: HTMLIonAlertElement;
alert = page.body.querySelector('ion-alert')!;
expect(alert).not.toBe(null);
expect(alert.getAttribute('id')).toBe('ion-overlay-1');
// Remove the alert from the DOM
alert.remove();
await page.waitForChanges();
// Create a new alert to verify the id is incremented
alert = document.createElement('ion-alert');
alert.isOpen = true;
page.body.appendChild(alert);
await page.waitForChanges();
alert = page.body.querySelector('ion-alert')!;
expect(alert.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same alert again should reuse the existing id
alert.isOpen = false;
await page.waitForChanges();
alert.isOpen = true;
await page.waitForChanges();
alert = page.body.querySelector('ion-alert')!;
expect(alert.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Alert } from '../alert';
import { h } from '@stencil/core';
describe('alert: id', () => {
it('alert should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Alert],
html: `<ion-alert is-open="true"></ion-alert>`,
});
let alert: HTMLIonAlertElement;
alert = page.body.querySelector('ion-alert')!;
expect(alert).not.toBe(null);
expect(alert.getAttribute('id')).toBe('ion-overlay-1');
// Remove the alert from the DOM
alert.remove();
await page.waitForChanges();
// Create a new alert to verify the id is incremented
alert = document.createElement('ion-alert');
alert.isOpen = true;
page.body.appendChild(alert);
await page.waitForChanges();
alert = page.body.querySelector('ion-alert')!;
expect(alert.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same alert again should reuse the existing id
alert.isOpen = false;
await page.waitForChanges();
alert.isOpen = true;
await page.waitForChanges();
alert = page.body.querySelector('ion-alert')!;
expect(alert.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Alert],
template: () => <ion-alert htmlAttributes={{ id }} overlayIndex={-1}></ion-alert>,
});
const alert = page.body.querySelector('ion-alert')!;
expect(alert.id).toBe(id);
});
});

View File

@ -2,7 +2,6 @@ import { newSpecPage } from '@stencil/core/testing';
import { config } from '../../../global/config';
import { Alert } from '../alert';
import { h } from '@stencil/core';
describe('alert: custom html', () => {
it('should not allow for custom html by default', async () => {
@ -39,15 +38,4 @@ describe('alert: custom html', () => {
expect(content.textContent).toContain('Custom Text');
expect(content.querySelector('button.custom-html')).toBe(null);
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Alert],
template: () => <ion-alert htmlAttributes={{ id }} overlayIndex={-1}></ion-alert>,
});
const alert = page.body.querySelector('ion-alert')!;
expect(alert.id).toBe(id);
});
});

View File

@ -214,7 +214,9 @@ export class Loading implements ComponentInterface, OverlayInterface {
const mode = getIonMode(this);
this.spinner = config.get('loadingSpinner', config.get('spinner', mode === 'ios' ? 'lines' : 'crescent'));
}
setOverlayId(this.el);
if (!this.htmlAttributes?.id) {
setOverlayId(this.el);
}
}
componentDidLoad() {

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Loading } from '../loading';
it('loading should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Loading],
html: `<ion-loading is-open="true"></ion-loading>`,
});
let loading: HTMLIonLoadingElement;
loading = page.body.querySelector('ion-loading')!;
expect(loading).not.toBe(null);
expect(loading.getAttribute('id')).toBe('ion-overlay-1');
// Remove the loading from the DOM
loading.remove();
await page.waitForChanges();
// Create a new loading to verify the id is incremented
loading = document.createElement('ion-loading');
loading.isOpen = true;
page.body.appendChild(loading);
await page.waitForChanges();
loading = page.body.querySelector('ion-loading')!;
expect(loading.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same loading again should reuse the existing id
loading.isOpen = false;
await page.waitForChanges();
loading.isOpen = true;
await page.waitForChanges();
loading = page.body.querySelector('ion-loading')!;
expect(loading.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Loading } from '../loading';
import { h } from '@stencil/core';
describe('loading: id', () => {
it('loading should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Loading],
html: `<ion-loading is-open="true"></ion-loading>`,
});
let loading: HTMLIonLoadingElement;
loading = page.body.querySelector('ion-loading')!;
expect(loading).not.toBe(null);
expect(loading.getAttribute('id')).toBe('ion-overlay-1');
// Remove the loading from the DOM
loading.remove();
await page.waitForChanges();
// Create a new loading to verify the id is incremented
loading = document.createElement('ion-loading');
loading.isOpen = true;
page.body.appendChild(loading);
await page.waitForChanges();
loading = page.body.querySelector('ion-loading')!;
expect(loading.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same loading again should reuse the existing id
loading.isOpen = false;
await page.waitForChanges();
loading.isOpen = true;
await page.waitForChanges();
loading = page.body.querySelector('ion-loading')!;
expect(loading.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Loading],
template: () => <ion-loading htmlAttributes={{ id }} overlayIndex={-1}></ion-loading>,
});
const alert = page.body.querySelector('ion-loading')!;
expect(alert.id).toBe(id);
});
});

View File

@ -415,7 +415,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
printIonWarning('Your breakpoints array must include the initialBreakpoint value.');
}
setOverlayId(el);
if (!this.htmlAttributes?.id) {
setOverlayId(this.el);
}
}
componentDidLoad() {

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Modal } from '../modal';
it('modal should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Modal],
html: `<ion-modal is-open="true"></ion-modal>`,
});
let modal: HTMLIonModalElement;
modal = page.body.querySelector('ion-modal')!;
expect(modal).not.toBe(null);
expect(modal.getAttribute('id')).toBe('ion-overlay-1');
// Remove the modal from the DOM
modal.remove();
await page.waitForChanges();
// Create a new modal to verify the id is incremented
modal = document.createElement('ion-modal');
modal.isOpen = true;
page.body.appendChild(modal);
await page.waitForChanges();
modal = page.body.querySelector('ion-modal')!;
expect(modal.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same modal again should reuse the existing id
modal.isOpen = false;
await page.waitForChanges();
modal.isOpen = true;
await page.waitForChanges();
modal = page.body.querySelector('ion-modal')!;
expect(modal.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Modal } from '../modal';
import { h } from '@stencil/core';
describe('modal: id', () => {
it('modal should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Modal],
html: `<ion-modal is-open="true"></ion-modal>`,
});
let modal: HTMLIonModalElement;
modal = page.body.querySelector('ion-modal')!;
expect(modal).not.toBe(null);
expect(modal.getAttribute('id')).toBe('ion-overlay-1');
// Remove the modal from the DOM
modal.remove();
await page.waitForChanges();
// Create a new modal to verify the id is incremented
modal = document.createElement('ion-modal');
modal.isOpen = true;
page.body.appendChild(modal);
await page.waitForChanges();
modal = page.body.querySelector('ion-modal')!;
expect(modal.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same modal again should reuse the existing id
modal.isOpen = false;
await page.waitForChanges();
modal.isOpen = true;
await page.waitForChanges();
modal = page.body.querySelector('ion-modal')!;
expect(modal.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Modal],
template: () => <ion-modal htmlAttributes={{ id }} overlayIndex={-1}></ion-modal>,
});
const alert = page.body.querySelector('ion-modal')!;
expect(alert.id).toBe(id);
});
});

View File

@ -199,7 +199,9 @@ export class Picker implements ComponentInterface, OverlayInterface {
}
componentWillLoad() {
setOverlayId(this.el);
if (!this.htmlAttributes?.id) {
setOverlayId(this.el);
}
}
componentDidLoad() {

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Picker } from '../picker';
it('picker should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Picker],
html: `<ion-picker-legacy is-open="true"></ion-picker-legacy>`,
});
let picker: HTMLIonPickerLegacyElement;
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker).not.toBe(null);
expect(picker.getAttribute('id')).toBe('ion-overlay-1');
// Remove the picker from the DOM
picker.remove();
await page.waitForChanges();
// Create a new picker to verify the id is incremented
picker = document.createElement('ion-picker-legacy');
picker.isOpen = true;
page.body.appendChild(picker);
await page.waitForChanges();
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same picker again should reuse the existing id
picker.isOpen = false;
await page.waitForChanges();
picker.isOpen = true;
await page.waitForChanges();
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Picker } from '../picker';
import { h } from '@stencil/core';
describe('picker: id', () => {
it('picker should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Picker],
html: `<ion-picker-legacy is-open="true"></ion-picker-legacy>`,
});
let picker: HTMLIonPickerLegacyElement;
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker).not.toBe(null);
expect(picker.getAttribute('id')).toBe('ion-overlay-1');
// Remove the picker from the DOM
picker.remove();
await page.waitForChanges();
// Create a new picker to verify the id is incremented
picker = document.createElement('ion-picker-legacy');
picker.isOpen = true;
page.body.appendChild(picker);
await page.waitForChanges();
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same picker again should reuse the existing id
picker.isOpen = false;
await page.waitForChanges();
picker.isOpen = true;
await page.waitForChanges();
picker = page.body.querySelector('ion-picker-legacy')!;
expect(picker.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Picker],
template: () => <ion-picker-legacy htmlAttributes={{ id }} overlayIndex={-1}></ion-picker-legacy>,
});
const alert = page.body.querySelector('ion-picker-legacy')!;
expect(alert.id).toBe(id);
});
});

View File

@ -365,7 +365,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
componentWillLoad() {
const { el } = this;
const popoverId = setOverlayId(el);
const popoverId = this.htmlAttributes?.id ?? setOverlayId(el);
this.parentPopover = el.closest(`ion-popover:not(#${popoverId})`) as HTMLIonPopoverElement | null;

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Popover } from '../popover';
it('popover should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Popover],
html: `<ion-popover is-open="true"></ion-popover>`,
});
let popover: HTMLIonPopoverElement;
popover = page.body.querySelector('ion-popover')!;
expect(popover).not.toBe(null);
expect(popover.getAttribute('id')).toBe('ion-overlay-1');
// Remove the popover from the DOM
popover.remove();
await page.waitForChanges();
// Create a new popover to verify the id is incremented
popover = document.createElement('ion-popover');
popover.isOpen = true;
page.body.appendChild(popover);
await page.waitForChanges();
popover = page.body.querySelector('ion-popover')!;
expect(popover.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same popover again should reuse the existing id
popover.isOpen = false;
await page.waitForChanges();
popover.isOpen = true;
await page.waitForChanges();
popover = page.body.querySelector('ion-popover')!;
expect(popover.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Popover } from '../popover';
import { h } from '@stencil/core';
describe('popover: id', () => {
it('popover should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Popover],
html: `<ion-popover is-open="true"></ion-popover>`,
});
let popover: HTMLIonPopoverElement;
popover = page.body.querySelector('ion-popover')!;
expect(popover).not.toBe(null);
expect(popover.getAttribute('id')).toBe('ion-overlay-1');
// Remove the popover from the DOM
popover.remove();
await page.waitForChanges();
// Create a new popover to verify the id is incremented
popover = document.createElement('ion-popover');
popover.isOpen = true;
page.body.appendChild(popover);
await page.waitForChanges();
popover = page.body.querySelector('ion-popover')!;
expect(popover.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same popover again should reuse the existing id
popover.isOpen = false;
await page.waitForChanges();
popover.isOpen = true;
await page.waitForChanges();
popover = page.body.querySelector('ion-popover')!;
expect(popover.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Popover],
template: () => <ion-popover htmlAttributes={{ id }} overlayIndex={-1}></ion-popover>,
});
const alert = page.body.querySelector('ion-popover')!;
expect(alert.id).toBe(id);
});
});

View File

@ -1,41 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Toast } from '../toast';
it('toast should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Toast],
html: `<ion-toast is-open="true"></ion-toast>`,
});
let toast: HTMLIonToastElement;
toast = page.body.querySelector('ion-toast')!;
expect(toast).not.toBe(null);
expect(toast.getAttribute('id')).toBe('ion-overlay-1');
// Remove the toast from the DOM
toast.remove();
await page.waitForChanges();
// Create a new toast to verify the id is incremented
toast = document.createElement('ion-toast');
toast.isOpen = true;
page.body.appendChild(toast);
await page.waitForChanges();
toast = page.body.querySelector('ion-toast')!;
expect(toast.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same toast again should reuse the existing id
toast.isOpen = false;
await page.waitForChanges();
toast.isOpen = true;
await page.waitForChanges();
toast = page.body.querySelector('ion-toast')!;
expect(toast.getAttribute('id')).toBe('ion-overlay-2');
});

View File

@ -0,0 +1,55 @@
import { newSpecPage } from '@stencil/core/testing';
import { Toast } from '../toast';
import { h } from '@stencil/core';
describe('toast: id', () => {
it('toast should be assigned an incrementing id', async () => {
const page = await newSpecPage({
components: [Toast],
html: `<ion-toast is-open="true"></ion-toast>`,
});
let toast: HTMLIonToastElement;
toast = page.body.querySelector('ion-toast')!;
expect(toast).not.toBe(null);
expect(toast.getAttribute('id')).toBe('ion-overlay-1');
// Remove the toast from the DOM
toast.remove();
await page.waitForChanges();
// Create a new toast to verify the id is incremented
toast = document.createElement('ion-toast');
toast.isOpen = true;
page.body.appendChild(toast);
await page.waitForChanges();
toast = page.body.querySelector('ion-toast')!;
expect(toast.getAttribute('id')).toBe('ion-overlay-2');
// Presenting the same toast again should reuse the existing id
toast.isOpen = false;
await page.waitForChanges();
toast.isOpen = true;
await page.waitForChanges();
toast = page.body.querySelector('ion-toast')!;
expect(toast.getAttribute('id')).toBe('ion-overlay-2');
});
it('should not overwrite the id set in htmlAttributes', async () => {
const id = 'custom-id';
const page = await newSpecPage({
components: [Toast],
template: () => <ion-toast htmlAttributes={{ id }} overlayIndex={-1}></ion-toast>,
});
const alert = page.body.querySelector('ion-toast')!;
expect(alert.id).toBe(id);
});
});

View File

@ -321,7 +321,9 @@ export class Toast implements ComponentInterface, OverlayInterface {
}
componentWillLoad() {
setOverlayId(this.el);
if (!this.htmlAttributes?.id) {
setOverlayId(this.el);
}
}
componentDidLoad() {