From 92ce563c402e557b08a622bd20f78459782312e2 Mon Sep 17 00:00:00 2001 From: Mikel Hamer <10012080+mikelhamer@users.noreply.github.com> Date: Wed, 24 Jul 2024 10:17:56 -0400 Subject: [PATCH] fix(overlays): do not overwrite id set in htmlAttributes (#29722) Issue number: resolves #29712 --------- ## 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 ## Other information --------- Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com> --- .../components/action-sheet/action-sheet.tsx | 4 +- .../action-sheet/test/action-sheet-id.spec.ts | 41 -------------- .../test/action-sheet-id.spec.tsx | 55 +++++++++++++++++++ .../components/alert/test/alert-id.spec.ts | 41 -------------- .../components/alert/test/alert-id.spec.tsx | 55 +++++++++++++++++++ .../test/{alert.spec.tsx => alert.spec.ts} | 12 ---- core/src/components/loading/loading.tsx | 4 +- .../loading/test/loading-id.spec.ts | 41 -------------- .../loading/test/loading-id.spec.tsx | 55 +++++++++++++++++++ core/src/components/modal/modal.tsx | 4 +- .../components/modal/test/modal-id.spec.ts | 41 -------------- .../components/modal/test/modal-id.spec.tsx | 55 +++++++++++++++++++ core/src/components/picker-legacy/picker.tsx | 4 +- .../picker-legacy/test/picker-id.spec.ts | 41 -------------- .../picker-legacy/test/picker-id.spec.tsx | 55 +++++++++++++++++++ core/src/components/popover/popover.tsx | 2 +- .../popover/test/popover-id.spec.ts | 41 -------------- .../popover/test/popover-id.spec.tsx | 55 +++++++++++++++++++ .../components/toast/test/toast-id.spec.ts | 41 -------------- .../components/toast/test/toast-id.spec.tsx | 55 +++++++++++++++++++ core/src/components/toast/toast.tsx | 4 +- 21 files changed, 401 insertions(+), 305 deletions(-) delete mode 100644 core/src/components/action-sheet/test/action-sheet-id.spec.ts create mode 100644 core/src/components/action-sheet/test/action-sheet-id.spec.tsx delete mode 100644 core/src/components/alert/test/alert-id.spec.ts create mode 100644 core/src/components/alert/test/alert-id.spec.tsx rename core/src/components/alert/test/{alert.spec.tsx => alert.spec.ts} (80%) delete mode 100644 core/src/components/loading/test/loading-id.spec.ts create mode 100644 core/src/components/loading/test/loading-id.spec.tsx delete mode 100644 core/src/components/modal/test/modal-id.spec.ts create mode 100644 core/src/components/modal/test/modal-id.spec.tsx delete mode 100644 core/src/components/picker-legacy/test/picker-id.spec.ts create mode 100644 core/src/components/picker-legacy/test/picker-id.spec.tsx delete mode 100644 core/src/components/popover/test/popover-id.spec.ts create mode 100644 core/src/components/popover/test/popover-id.spec.tsx delete mode 100644 core/src/components/toast/test/toast-id.spec.ts create mode 100644 core/src/components/toast/test/toast-id.spec.tsx diff --git a/core/src/components/action-sheet/action-sheet.tsx b/core/src/components/action-sheet/action-sheet.tsx index 12b803f549..2d003a5dfc 100644 --- a/core/src/components/action-sheet/action-sheet.tsx +++ b/core/src/components/action-sheet/action-sheet.tsx @@ -310,7 +310,9 @@ export class ActionSheet implements ComponentInterface, OverlayInterface { } componentWillLoad() { - setOverlayId(this.el); + if (!this.htmlAttributes?.id) { + setOverlayId(this.el); + } } componentDidLoad() { diff --git a/core/src/components/action-sheet/test/action-sheet-id.spec.ts b/core/src/components/action-sheet/test/action-sheet-id.spec.ts deleted file mode 100644 index e5503b7eb1..0000000000 --- a/core/src/components/action-sheet/test/action-sheet-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/action-sheet/test/action-sheet-id.spec.tsx b/core/src/components/action-sheet/test/action-sheet-id.spec.tsx new file mode 100644 index 0000000000..63e7e17b1e --- /dev/null +++ b/core/src/components/action-sheet/test/action-sheet-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-action-sheet')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/alert/test/alert-id.spec.ts b/core/src/components/alert/test/alert-id.spec.ts deleted file mode 100644 index 25c1427b1b..0000000000 --- a/core/src/components/alert/test/alert-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/alert/test/alert-id.spec.tsx b/core/src/components/alert/test/alert-id.spec.tsx new file mode 100644 index 0000000000..b6ff5372e0 --- /dev/null +++ b/core/src/components/alert/test/alert-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-alert')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/alert/test/alert.spec.tsx b/core/src/components/alert/test/alert.spec.ts similarity index 80% rename from core/src/components/alert/test/alert.spec.tsx rename to core/src/components/alert/test/alert.spec.ts index 31c3872e6c..d7c8abb25c 100644 --- a/core/src/components/alert/test/alert.spec.tsx +++ b/core/src/components/alert/test/alert.spec.ts @@ -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: () => , - }); - - const alert = page.body.querySelector('ion-alert')!; - expect(alert.id).toBe(id); - }); }); diff --git a/core/src/components/loading/loading.tsx b/core/src/components/loading/loading.tsx index ceb2ae62a3..dac4cc5faf 100644 --- a/core/src/components/loading/loading.tsx +++ b/core/src/components/loading/loading.tsx @@ -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() { diff --git a/core/src/components/loading/test/loading-id.spec.ts b/core/src/components/loading/test/loading-id.spec.ts deleted file mode 100644 index f6b10e079b..0000000000 --- a/core/src/components/loading/test/loading-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/loading/test/loading-id.spec.tsx b/core/src/components/loading/test/loading-id.spec.tsx new file mode 100644 index 0000000000..22e963e97a --- /dev/null +++ b/core/src/components/loading/test/loading-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-loading')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx index 7a3fd4b047..bc8f6184f9 100644 --- a/core/src/components/modal/modal.tsx +++ b/core/src/components/modal/modal.tsx @@ -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() { diff --git a/core/src/components/modal/test/modal-id.spec.ts b/core/src/components/modal/test/modal-id.spec.ts deleted file mode 100644 index 4568aa6e1c..0000000000 --- a/core/src/components/modal/test/modal-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/modal/test/modal-id.spec.tsx b/core/src/components/modal/test/modal-id.spec.tsx new file mode 100644 index 0000000000..43f1a9eaa1 --- /dev/null +++ b/core/src/components/modal/test/modal-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-modal')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/picker-legacy/picker.tsx b/core/src/components/picker-legacy/picker.tsx index d050714a71..cb6f1ba2d9 100644 --- a/core/src/components/picker-legacy/picker.tsx +++ b/core/src/components/picker-legacy/picker.tsx @@ -199,7 +199,9 @@ export class Picker implements ComponentInterface, OverlayInterface { } componentWillLoad() { - setOverlayId(this.el); + if (!this.htmlAttributes?.id) { + setOverlayId(this.el); + } } componentDidLoad() { diff --git a/core/src/components/picker-legacy/test/picker-id.spec.ts b/core/src/components/picker-legacy/test/picker-id.spec.ts deleted file mode 100644 index d63fb65ce8..0000000000 --- a/core/src/components/picker-legacy/test/picker-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/picker-legacy/test/picker-id.spec.tsx b/core/src/components/picker-legacy/test/picker-id.spec.tsx new file mode 100644 index 0000000000..6980cc347d --- /dev/null +++ b/core/src/components/picker-legacy/test/picker-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-picker-legacy')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/popover/popover.tsx b/core/src/components/popover/popover.tsx index 4506e3b7ca..d1205ab2f6 100644 --- a/core/src/components/popover/popover.tsx +++ b/core/src/components/popover/popover.tsx @@ -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; diff --git a/core/src/components/popover/test/popover-id.spec.ts b/core/src/components/popover/test/popover-id.spec.ts deleted file mode 100644 index fb42e12b40..0000000000 --- a/core/src/components/popover/test/popover-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/popover/test/popover-id.spec.tsx b/core/src/components/popover/test/popover-id.spec.tsx new file mode 100644 index 0000000000..05457fb1dd --- /dev/null +++ b/core/src/components/popover/test/popover-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-popover')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/toast/test/toast-id.spec.ts b/core/src/components/toast/test/toast-id.spec.ts deleted file mode 100644 index 79364d5691..0000000000 --- a/core/src/components/toast/test/toast-id.spec.ts +++ /dev/null @@ -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: ``, - }); - 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'); -}); diff --git a/core/src/components/toast/test/toast-id.spec.tsx b/core/src/components/toast/test/toast-id.spec.tsx new file mode 100644 index 0000000000..e993cfc33e --- /dev/null +++ b/core/src/components/toast/test/toast-id.spec.tsx @@ -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: ``, + }); + 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: () => , + }); + + const alert = page.body.querySelector('ion-toast')!; + expect(alert.id).toBe(id); + }); +}); diff --git a/core/src/components/toast/toast.tsx b/core/src/components/toast/toast.tsx index cf9bf1d77f..d006471618 100644 --- a/core/src/components/toast/toast.tsx +++ b/core/src/components/toast/toast.tsx @@ -321,7 +321,9 @@ export class Toast implements ComponentInterface, OverlayInterface { } componentWillLoad() { - setOverlayId(this.el); + if (!this.htmlAttributes?.id) { + setOverlayId(this.el); + } } componentDidLoad() {