diff --git a/angular/test/base/e2e/src/modal.spec.ts b/angular/test/base/e2e/src/modal.spec.ts index 049b83020f..153da005ad 100644 --- a/angular/test/base/e2e/src/modal.spec.ts +++ b/angular/test/base/e2e/src/modal.spec.ts @@ -121,4 +121,4 @@ describe('when in a modal', () => { cy.get('#set-to-null').click(); cy.get('#inputWithFloatingLabel').should('not.have.class', 'item-has-value'); }); -}); +}); \ No newline at end of file diff --git a/angular/test/base/src/app/modal-inline/modal-inline.component.html b/angular/test/base/src/app/modal-inline/modal-inline.component.html index b11f288cd7..2d14d2e545 100644 --- a/angular/test/base/src/app/modal-inline/modal-inline.component.html +++ b/angular/test/base/src/app/modal-inline/modal-inline.component.html @@ -17,4 +17,4 @@ - + \ No newline at end of file diff --git a/angular/test/base/src/app/modal-inline/modal-inline.component.ts b/angular/test/base/src/app/modal-inline/modal-inline.component.ts index 8ba5878bfc..b695291f10 100644 --- a/angular/test/base/src/app/modal-inline/modal-inline.component.ts +++ b/angular/test/base/src/app/modal-inline/modal-inline.component.ts @@ -24,4 +24,5 @@ export class ModalInlineComponent implements AfterViewInit { onBreakpointDidChange() { this.breakpointDidChangeCounter++; } + } diff --git a/core/src/components/action-sheet/action-sheet.tsx b/core/src/components/action-sheet/action-sheet.tsx index 56732bb0cb..3a13827a3c 100644 --- a/core/src/components/action-sheet/action-sheet.tsx +++ b/core/src/components/action-sheet/action-sheet.tsx @@ -15,6 +15,7 @@ import { prepareOverlay, present, safeCall, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import { getClassMap } from '../../utils/theme'; @@ -311,6 +312,10 @@ export class ActionSheet implements ComponentInterface, OverlayInterface { this.triggerController.removeClickListener(); } + componentWillLoad() { + setOverlayId(this.el); + } + componentDidLoad() { /** * Do not create gesture if: 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 new file mode 100644 index 0000000000..e5503b7eb1 --- /dev/null +++ b/core/src/components/action-sheet/test/action-sheet-id.spec.ts @@ -0,0 +1,41 @@ +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/alert/alert.tsx b/core/src/components/alert/alert.tsx index 83b945c6b0..7c00a569d8 100644 --- a/core/src/components/alert/alert.tsx +++ b/core/src/components/alert/alert.tsx @@ -17,6 +17,7 @@ import { prepareOverlay, present, safeCall, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import type { IonicSafeString } from '../../utils/sanitization'; @@ -329,6 +330,7 @@ export class Alert implements ComponentInterface, OverlayInterface { } componentWillLoad() { + setOverlayId(this.el); this.inputsChanged(); this.buttonsChanged(); } diff --git a/core/src/components/alert/test/alert-id.spec.ts b/core/src/components/alert/test/alert-id.spec.ts new file mode 100644 index 0000000000..25c1427b1b --- /dev/null +++ b/core/src/components/alert/test/alert-id.spec.ts @@ -0,0 +1,41 @@ +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/loading/loading.tsx b/core/src/components/loading/loading.tsx index 13cb5c5c19..c8a04dc69c 100644 --- a/core/src/components/loading/loading.tsx +++ b/core/src/components/loading/loading.tsx @@ -14,6 +14,7 @@ import { present, createDelegateController, createTriggerController, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import type { IonicSafeString } from '../../utils/sanitization'; @@ -212,6 +213,7 @@ 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); } componentDidLoad() { diff --git a/core/src/components/loading/test/loading-id.spec.ts b/core/src/components/loading/test/loading-id.spec.ts new file mode 100644 index 0000000000..f6b10e079b --- /dev/null +++ b/core/src/components/loading/test/loading-id.spec.ts @@ -0,0 +1,41 @@ +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.spec.ts b/core/src/components/loading/test/loading.spec.ts index 152ed87b09..11fa9658d7 100644 --- a/core/src/components/loading/test/loading.spec.ts +++ b/core/src/components/loading/test/loading.spec.ts @@ -1,15 +1,16 @@ import { newSpecPage } from '@stencil/core/testing'; -import { Loading } from '../loading'; -import { config } from '../../../global/config'; -describe('alert: custom html', () => { +import { config } from '../../../global/config'; +import { Loading } from '../loading'; + +describe('loading: custom html', () => { it('should not allow for custom html by default', async () => { const page = await newSpecPage({ components: [Loading], html: ``, }); - const content = page.body.querySelector('.loading-content'); + const content = page.body.querySelector('.loading-content')!; expect(content.textContent).toContain('Custom Text'); expect(content.querySelector('button.custom-html')).toBe(null); }); @@ -21,7 +22,7 @@ describe('alert: custom html', () => { html: ``, }); - const content = page.body.querySelector('.loading-content'); + const content = page.body.querySelector('.loading-content')!; expect(content.textContent).toContain('Custom Text'); expect(content.querySelector('button.custom-html')).not.toBe(null); }); @@ -33,7 +34,7 @@ describe('alert: custom html', () => { html: ``, }); - const content = page.body.querySelector('.loading-content'); + const content = page.body.querySelector('.loading-content')!; expect(content.textContent).toContain('Custom Text'); expect(content.querySelector('button.custom-html')).toBe(null); }); diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx index 816484ab7f..2960b57272 100644 --- a/core/src/components/modal/modal.tsx +++ b/core/src/components/modal/modal.tsx @@ -28,6 +28,7 @@ import { prepareOverlay, present, createTriggerController, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import { getClassMap } from '../../utils/theme'; @@ -65,8 +66,6 @@ import { setCardStatusBarDark, setCardStatusBarDefault } from './utils'; export class Modal implements ComponentInterface, OverlayInterface { private readonly triggerController = createTriggerController(); private gesture?: Gesture; - private modalIndex = modalIds++; - private modalId?: string; private coreDelegate: FrameworkDelegate = CoreDelegate(); private currentTransition?: Promise; private sheetTransition?: Promise; @@ -344,16 +343,10 @@ export class Modal implements ComponentInterface, OverlayInterface { componentWillLoad() { const { breakpoints, initialBreakpoint, el } = this; + const isSheetModal = (this.isSheetModal = breakpoints !== undefined && initialBreakpoint !== undefined); this.inheritedAttributes = inheritAttributes(el, ['aria-label', 'role']); - /** - * If user has custom ID set then we should - * not assign the default incrementing ID. - */ - this.modalId = this.el.hasAttribute('id') ? this.el.getAttribute('id')! : `ion-modal-${this.modalIndex}`; - const isSheetModal = (this.isSheetModal = breakpoints !== undefined && initialBreakpoint !== undefined); - if (isSheetModal) { this.currentBreakpoint = this.initialBreakpoint; } @@ -361,6 +354,8 @@ export class Modal implements ComponentInterface, OverlayInterface { if (breakpoints !== undefined && initialBreakpoint !== undefined && !breakpoints.includes(initialBreakpoint)) { printIonWarning('Your breakpoints array must include the initialBreakpoint value.'); } + + setOverlayId(el); } componentDidLoad() { @@ -861,7 +856,6 @@ export class Modal implements ComponentInterface, OverlayInterface { const showHandle = handle !== false && isSheetModal; const mode = getIonMode(this); - const { modalId } = this; const isCardModal = presentingElement !== undefined && mode === 'ios'; const isHandleCycle = handleBehavior === 'cycle'; @@ -881,7 +875,6 @@ export class Modal implements ComponentInterface, OverlayInterface { 'overlay-hidden': true, ...getClassMap(this.cssClass), }} - id={modalId} onIonBackdropTap={this.onBackdropTap} onIonModalDidPresent={this.onLifecycle} onIonModalWillPresent={this.onLifecycle} @@ -935,8 +928,6 @@ const LIFECYCLE_MAP: any = { ionModalDidDismiss: 'ionViewDidLeave', }; -let modalIds = 0; - interface ModalOverlayOptions { /** * The element that presented the modal. diff --git a/core/src/components/modal/test/modal-id.spec.ts b/core/src/components/modal/test/modal-id.spec.ts new file mode 100644 index 0000000000..4568aa6e1c --- /dev/null +++ b/core/src/components/modal/test/modal-id.spec.ts @@ -0,0 +1,41 @@ +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/picker/picker.tsx b/core/src/components/picker/picker.tsx index 049dae6a20..881b4e7835 100644 --- a/core/src/components/picker/picker.tsx +++ b/core/src/components/picker/picker.tsx @@ -13,6 +13,7 @@ import { prepareOverlay, present, safeCall, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import { getClassMap } from '../../utils/theme'; @@ -194,6 +195,10 @@ export class Picker implements ComponentInterface, OverlayInterface { this.triggerController.removeClickListener(); } + componentWillLoad() { + setOverlayId(this.el); + } + /** * Present the picker overlay after it has been created. */ diff --git a/core/src/components/picker/test/picker-id.spec.ts b/core/src/components/picker/test/picker-id.spec.ts new file mode 100644 index 0000000000..e496e5b28a --- /dev/null +++ b/core/src/components/picker/test/picker-id.spec.ts @@ -0,0 +1,41 @@ +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: HTMLIonPickerElement; + + picker = page.body.querySelector('ion-picker')!; + + 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'); + picker.isOpen = true; + page.body.appendChild(picker); + await page.waitForChanges(); + + picker = page.body.querySelector('ion-picker')!; + + 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')!; + + expect(picker.getAttribute('id')).toBe('ion-overlay-2'); +}); diff --git a/core/src/components/popover/popover.tsx b/core/src/components/popover/popover.tsx index ff5779d5a2..ad7b294a8a 100644 --- a/core/src/components/popover/popover.tsx +++ b/core/src/components/popover/popover.tsx @@ -6,7 +6,15 @@ import type { AnimationBuilder, ComponentProps, ComponentRef, FrameworkDelegate import { CoreDelegate, attachComponent, detachComponent } from '../../utils/framework-delegate'; import { addEventListener, raf, hasLazyBuild } from '../../utils/helpers'; import { printIonWarning } from '../../utils/logging'; -import { BACKDROP, dismiss, eventMethod, focusFirstDescendant, prepareOverlay, present } from '../../utils/overlays'; +import { + BACKDROP, + dismiss, + eventMethod, + focusFirstDescendant, + prepareOverlay, + present, + setOverlayId, +} from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import { isPlatform } from '../../utils/platform'; import { getClassMap } from '../../utils/theme'; @@ -49,8 +57,6 @@ export class Popover implements ComponentInterface, PopoverInterface { private usersElement?: HTMLElement; private triggerEl?: HTMLElement | null; private parentPopover: HTMLIonPopoverElement | null = null; - private popoverIndex = popoverIds++; - private popoverId?: string; private coreDelegate: FrameworkDelegate = CoreDelegate(); private currentTransition?: Promise; private destroyTriggerInteraction?: () => void; @@ -338,13 +344,10 @@ export class Popover implements ComponentInterface, PopoverInterface { } componentWillLoad() { - /** - * If user has custom ID set then we should - * not assign the default incrementing ID. - */ - this.popoverId = this.el.hasAttribute('id') ? this.el.getAttribute('id')! : `ion-popover-${this.popoverIndex}`; + const { el } = this; + const popoverId = setOverlayId(el); - this.parentPopover = this.el.closest(`ion-popover:not(#${this.popoverId})`) as HTMLIonPopoverElement | null; + this.parentPopover = el.closest(`ion-popover:not(#${popoverId})`) as HTMLIonPopoverElement | null; if (this.alignment === undefined) { this.alignment = getIonMode(this) === 'ios' ? 'center' : 'start'; @@ -660,7 +663,7 @@ export class Popover implements ComponentInterface, PopoverInterface { render() { const mode = getIonMode(this); - const { onLifecycle, popoverId, parentPopover, dismissOnSelect, side, arrow, htmlAttributes } = this; + const { onLifecycle, parentPopover, dismissOnSelect, side, arrow, htmlAttributes } = this; const desktop = isPlatform('desktop'); const enableArrow = arrow && !parentPopover; @@ -673,7 +676,6 @@ export class Popover implements ComponentInterface, PopoverInterface { style={{ zIndex: `${20000 + this.overlayIndex}`, }} - id={popoverId} class={{ ...getClassMap(this.cssClass), [mode]: true, @@ -709,8 +711,6 @@ const LIFECYCLE_MAP: any = { ionPopoverDidDismiss: 'ionViewDidLeave', }; -let popoverIds = 0; - interface PopoverPresentOptions { /** * The original target event that presented the popover. diff --git a/core/src/components/popover/test/popover-id.spec.ts b/core/src/components/popover/test/popover-id.spec.ts new file mode 100644 index 0000000000..fb42e12b40 --- /dev/null +++ b/core/src/components/popover/test/popover-id.spec.ts @@ -0,0 +1,41 @@ +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/toast/test/toast-id.spec.ts b/core/src/components/toast/test/toast-id.spec.ts new file mode 100644 index 0000000000..79364d5691 --- /dev/null +++ b/core/src/components/toast/test/toast-id.spec.ts @@ -0,0 +1,41 @@ +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/toast.tsx b/core/src/components/toast/toast.tsx index 9c2f01ce9d..4e43505d40 100644 --- a/core/src/components/toast/toast.tsx +++ b/core/src/components/toast/toast.tsx @@ -15,6 +15,7 @@ import { prepareOverlay, present, safeCall, + setOverlayId, } from '../../utils/overlays'; import type { OverlayEventDetail } from '../../utils/overlays-interface'; import type { IonicSafeString } from '../../utils/sanitization'; @@ -248,6 +249,10 @@ export class Toast implements ComponentInterface, OverlayInterface { this.triggerController.removeClickListener(); } + componentWillLoad() { + setOverlayId(this.el); + } + /** * Present the toast overlay after it has been created. */ diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts index 69fb68f3de..3f8c83d59d 100644 --- a/core/src/utils/overlays.ts +++ b/core/src/utils/overlays.ts @@ -22,6 +22,7 @@ import { OVERLAY_BACK_BUTTON_PRIORITY } from './hardware-back-button'; import { addEventListener, componentOnReady, focusElement, getElementRoot, removeEventListener } from './helpers'; import { printIonWarning } from './logging'; +let lastOverlayIndex = 0; let lastId = 0; export const activeAnimations = new WeakMap(); @@ -50,15 +51,42 @@ export const pickerController = /*@__PURE__*/ createController('ion-popover'); export const toastController = /*@__PURE__*/ createController('ion-toast'); +/** + * Prepares the overlay element to be presented. + */ export const prepareOverlay = (el: T) => { if (typeof document !== 'undefined') { + /** + * Adds a single instance of event listeners for application behaviors: + * + * - Escape Key behavior to dismiss an overlay + * - Trapping focus within an overlay + * - Back button behavior to dismiss an overlay + * + * This only occurs when the first overlay is created. + */ connectListeners(document); } - const overlayIndex = lastId++; + const overlayIndex = lastOverlayIndex++; + /** + * overlayIndex is used in the overlay components to set a zIndex. + * This ensures that the most recently presented overlay will be + * on top. + */ el.overlayIndex = overlayIndex; +}; + +/** + * Assigns an incrementing id to an overlay element, that does not + * already have an id assigned to it. + * + * Used to track unique instances of an overlay element. + */ +export const setOverlayId = (el: T) => { if (!el.hasAttribute('id')) { - el.id = `ion-overlay-${overlayIndex}`; + el.id = `ion-overlay-${++lastId}`; } + return el.id; }; export const createOverlay = ( @@ -301,8 +329,8 @@ const trapKeyboardFocus = (ev: Event, doc: Document) => { }; const connectListeners = (doc: Document) => { - if (lastId === 0) { - lastId = 1; + if (lastOverlayIndex === 0) { + lastOverlayIndex = 1; doc.addEventListener( 'focus', (ev: FocusEvent) => {