From 80ee5f7ab9764fa60532f259b4772263cd06414a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Louren=C3=A7o?= Date: Thu, 16 Jan 2025 16:04:56 +0000 Subject: [PATCH 1/5] fix(segment-button): protect connectedCallback for when segment-content has not yet been created (#30133) Issue number: internal --------- ## What is the current behavior? When the `connectedCallback` method is called for a segment-button and its corresponding segment-content has not been created in that instant, a console error is thrown and the method returns. ## What is the new behavior? - `connectedCallback` will now wait, at most 1 second, for the corresponding segment-content to be created. - The new behaviour can be tested in segment-view/basic. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information --- .../segment-button/segment-button.tsx | 60 ++++++++++++++++--- .../segment-view/test/basic/index.html | 30 ++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/core/src/components/segment-button/segment-button.tsx b/core/src/components/segment-button/segment-button.tsx index d860735f729..90c9107d30f 100644 --- a/core/src/components/segment-button/segment-button.tsx +++ b/core/src/components/segment-button/segment-button.tsx @@ -65,7 +65,52 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { this.updateState(); } - connectedCallback() { + private getNextSiblingOfType(element: Element): T | null { + let sibling = element.nextSibling; + while (sibling) { + if (sibling.nodeType === Node.ELEMENT_NODE && (sibling as T) !== null) { + return sibling as T; + } + sibling = sibling.nextSibling; + } + return null; + } + + private waitForSegmentContent(ionSegment: HTMLIonSegmentElement | null, contentId: string): Promise { + return new Promise((resolve, reject) => { + let timeoutId: NodeJS.Timeout | undefined = undefined; + let animationFrameId: number; + + const check = () => { + if (!ionSegment) { + reject(new Error(`Segment not found when looking for Segment Content`)); + return; + } + + const segmentView = this.getNextSiblingOfType(ionSegment); // Skip the text nodes + const segmentContent = segmentView?.querySelector( + `ion-segment-content[id="${contentId}"]` + ) as HTMLIonSegmentContentElement | null; + if (segmentContent) { + clearTimeout(timeoutId); // Clear the timeout if the segmentContent is found + cancelAnimationFrame(animationFrameId); + resolve(segmentContent); + } else { + animationFrameId = requestAnimationFrame(check); // Keep checking on the next animation frame + } + }; + + check(); + + // Set a timeout to reject the promise + timeoutId = setTimeout(() => { + cancelAnimationFrame(animationFrameId); + reject(new Error(`Unable to find Segment Content with id="${contentId} within 1000 ms`)); + }, 1000); + }); + } + + async connectedCallback() { const segmentEl = (this.segmentEl = this.el.closest('ion-segment')); if (segmentEl) { this.updateState(); @@ -76,12 +121,13 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { // Return if there is no contentId defined if (!this.contentId) return; - // Attempt to find the Segment Content by its contentId - const segmentContent = document.getElementById(this.contentId) as HTMLIonSegmentContentElement | null; - - // If no associated Segment Content exists, log an error and return - if (!segmentContent) { - console.error(`Segment Button: Unable to find Segment Content with id="${this.contentId}".`); + let segmentContent; + try { + // Attempt to find the Segment Content by its contentId + segmentContent = await this.waitForSegmentContent(segmentEl, this.contentId); + } catch (error) { + // If no associated Segment Content exists, log an error and return + console.error('Segment Button: ', (error as Error).message); return; } diff --git a/core/src/components/segment-view/test/basic/index.html b/core/src/components/segment-view/test/basic/index.html index 69d36d4a6c0..78dec1d9ff9 100644 --- a/core/src/components/segment-view/test/basic/index.html +++ b/core/src/components/segment-view/test/basic/index.html @@ -123,6 +123,8 @@ + + @@ -158,6 +160,34 @@ segment.value = undefined; }); } + + async function addSegmentButtonAndContent() { + const segment = document.querySelector('ion-segment'); + const segmentView = document.querySelector('ion-segment-view'); + + const newButton = document.createElement('ion-segment-button'); + const newId = `new-${Date.now()}`; + newButton.setAttribute('content-id', newId); + newButton.setAttribute('value', newId); + newButton.innerHTML = 'New Button'; + + segment.appendChild(newButton); + + setTimeout(() => { + // Timeout to test waitForSegmentContent() in segment-button + const newContent = document.createElement('ion-segment-content'); + newContent.setAttribute('id', newId); + newContent.innerHTML = 'New Content'; + + segmentView.appendChild(newContent); + + // Necessary timeout to ensure the value is set after the content is added. + // Otherwise, the transition is unsuccessful and the content is not shown. + setTimeout(() => { + segment.setAttribute('value', newId); + }, 200); + }, 200); + } From 01b622e3f0e24753d9020fbc1310108a0f75e7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Louren=C3=A7o?= Date: Fri, 17 Jan 2025 13:03:02 +0000 Subject: [PATCH 2/5] Fix build --- core/src/utils/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 97681f6652a..fdf351e1a0c 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -22,7 +22,7 @@ export const transitionEndAsync = (el: HTMLElement | null, expectedDuration = 0) */ const transitionEnd = (el: HTMLElement | null, expectedDuration = 0, callback: (ev?: TransitionEvent) => void) => { let unRegTrans: (() => void) | undefined; - let animationTimeout: number | undefined; + let animationTimeout: NodeJS.Timeout | undefined; const opts: AddEventListenerOptions = { passive: true }; const ANIMATION_FALLBACK_TIMEOUT = 500; From c13ed3328c333e052f46c4ef5e9377ff99655170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Louren=C3=A7o?= Date: Fri, 17 Jan 2025 13:27:33 +0000 Subject: [PATCH 3/5] Fix build (second attempt) --- core/src/components/segment-button/segment-button.tsx | 2 +- core/src/utils/helpers.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/segment-button/segment-button.tsx b/core/src/components/segment-button/segment-button.tsx index 90c9107d30f..427e522cfa1 100644 --- a/core/src/components/segment-button/segment-button.tsx +++ b/core/src/components/segment-button/segment-button.tsx @@ -91,7 +91,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { const segmentContent = segmentView?.querySelector( `ion-segment-content[id="${contentId}"]` ) as HTMLIonSegmentContentElement | null; - if (segmentContent) { + if (segmentContent && timeoutId) { clearTimeout(timeoutId); // Clear the timeout if the segmentContent is found cancelAnimationFrame(animationFrameId); resolve(segmentContent); diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index fdf351e1a0c..97681f6652a 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -22,7 +22,7 @@ export const transitionEndAsync = (el: HTMLElement | null, expectedDuration = 0) */ const transitionEnd = (el: HTMLElement | null, expectedDuration = 0, callback: (ev?: TransitionEvent) => void) => { let unRegTrans: (() => void) | undefined; - let animationTimeout: NodeJS.Timeout | undefined; + let animationTimeout: number | undefined; const opts: AddEventListenerOptions = { passive: true }; const ANIMATION_FALLBACK_TIMEOUT = 500; From bc7c5a9f9ea9e86f116a802d10136d0278d74594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Louren=C3=A7o?= Date: Mon, 3 Feb 2025 11:03:23 +0000 Subject: [PATCH 4/5] CR --- .../components/segment-button/segment-button.tsx | 15 ++------------- core/src/utils/helpers.ts | 13 ++++++++++++- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/src/components/segment-button/segment-button.tsx b/core/src/components/segment-button/segment-button.tsx index 427e522cfa1..3a2135421c2 100644 --- a/core/src/components/segment-button/segment-button.tsx +++ b/core/src/components/segment-button/segment-button.tsx @@ -2,7 +2,7 @@ import type { ComponentInterface } from '@stencil/core'; import { Component, Element, Host, Prop, Method, State, Watch, forceUpdate, h } from '@stencil/core'; import type { ButtonInterface } from '@utils/element-interface'; import type { Attributes } from '@utils/helpers'; -import { addEventListener, removeEventListener, inheritAttributes } from '@utils/helpers'; +import { addEventListener, removeEventListener, inheritAttributes, getNextSiblingOfType } from '@utils/helpers'; import { hostContext } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -65,17 +65,6 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { this.updateState(); } - private getNextSiblingOfType(element: Element): T | null { - let sibling = element.nextSibling; - while (sibling) { - if (sibling.nodeType === Node.ELEMENT_NODE && (sibling as T) !== null) { - return sibling as T; - } - sibling = sibling.nextSibling; - } - return null; - } - private waitForSegmentContent(ionSegment: HTMLIonSegmentElement | null, contentId: string): Promise { return new Promise((resolve, reject) => { let timeoutId: NodeJS.Timeout | undefined = undefined; @@ -87,7 +76,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { return; } - const segmentView = this.getNextSiblingOfType(ionSegment); // Skip the text nodes + const segmentView = getNextSiblingOfType(ionSegment); // Skip the text nodes const segmentContent = segmentView?.querySelector( `ion-segment-content[id="${contentId}"]` ) as HTMLIonSegmentContentElement | null; diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 97681f6652a..5e8f02293ae 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -22,7 +22,7 @@ export const transitionEndAsync = (el: HTMLElement | null, expectedDuration = 0) */ const transitionEnd = (el: HTMLElement | null, expectedDuration = 0, callback: (ev?: TransitionEvent) => void) => { let unRegTrans: (() => void) | undefined; - let animationTimeout: number | undefined; + let animationTimeout: NodeJS.Timeout | undefined; const opts: AddEventListenerOptions = { passive: true }; const ANIMATION_FALLBACK_TIMEOUT = 500; @@ -413,3 +413,14 @@ export const shallowEqualStringMap = ( return true; }; + +export const getNextSiblingOfType = (element: Element): T | null => { + let sibling = element.nextSibling; + while (sibling) { + if (sibling.nodeType === Node.ELEMENT_NODE && (sibling as T) !== null) { + return sibling as T; + } + sibling = sibling.nextSibling; + } + return null; +}; From 2602bd34f852f755a0bdd7eacbb2202be450b0f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Louren=C3=A7o?= Date: Mon, 3 Feb 2025 11:06:10 +0000 Subject: [PATCH 5/5] Fix build --- core/src/utils/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 5e8f02293ae..a740ff98ac7 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -22,7 +22,7 @@ export const transitionEndAsync = (el: HTMLElement | null, expectedDuration = 0) */ const transitionEnd = (el: HTMLElement | null, expectedDuration = 0, callback: (ev?: TransitionEvent) => void) => { let unRegTrans: (() => void) | undefined; - let animationTimeout: NodeJS.Timeout | undefined; + let animationTimeout: number | undefined; const opts: AddEventListenerOptions = { passive: true }; const ANIMATION_FALLBACK_TIMEOUT = 500;