From 1cfa915e8fe362951c521bce970a9f5f10918ab2 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Tue, 25 Mar 2025 11:21:31 -0700 Subject: [PATCH] fix(segment-button): ensure consistent disabled state for segment-content error handling (#30288) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue number: N/A --------- ## What is the current behavior? Segment buttons do not consistently set themselves to a disabled state. When disabling them and rapidly refreshing the page, their state may vary—sometimes they appear disabled, and other times they do not. This was due to this [PR](https://github.com/ionic-team/ionic-framework/pull/30138). The reason that this PR was created was due to the console errors being shown too early when segment buttons and contents were dynamically rendered. ## What is the new behavior? - I reverted the changes added through the other PR since the `setTimeout` was causing the inconsistency. - By moving the console errors to `componentWillLoad`, it provides `ion-segment-content` time to render before the console errors are thrown. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `8.5.2-dev.11742514102.1b51674d` How to test: 1. Run the server locally 2. Navigate to the [segment view disabled](http://localhost:3333/src/components/segment-view/test/disabled) page 3. Verify that the "Paid", "Free", and "Top" buttons disabled 4. Rapid fire some hard refreshes 5. Verify that the "Paid", "Free", and "Top" buttons disabled 6. Navigate to this StackBlitz [repro](https://stackblitz.com/edit/yhktpj19-wxmxpmw7?file=src%2Fmain.tsx) 7. Install the dev build: `npm i @ionic/react@8.5.2-dev.11742514102.1b51674d @ionic/react-router@8.5.2-dev.11742514102.1b51674d` 8. Open the console log 9. Click on the "Add last child" button 10. Verify that there are no console errors like `Segment Button: Unable to find Segment Content with id="content3".` --- .../segment-button/segment-button.tsx | 77 +++++-------------- core/src/utils/helpers.ts | 11 --- 2 files changed, 21 insertions(+), 67 deletions(-) diff --git a/core/src/components/segment-button/segment-button.tsx b/core/src/components/segment-button/segment-button.tsx index 3a2135421c..fb940c9d99 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, getNextSiblingOfType } from '@utils/helpers'; +import { addEventListener, removeEventListener, inheritAttributes } from '@utils/helpers'; import { hostContext } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -65,41 +65,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { this.updateState(); } - 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 = getNextSiblingOfType(ionSegment); // Skip the text nodes - const segmentContent = segmentView?.querySelector( - `ion-segment-content[id="${contentId}"]` - ) as HTMLIonSegmentContentElement | null; - if (segmentContent && timeoutId) { - 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() { + connectedCallback() { const segmentEl = (this.segmentEl = this.el.closest('ion-segment')); if (segmentEl) { this.updateState(); @@ -107,27 +73,8 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { addEventListener(segmentEl, 'ionStyle', this.updateStyle); } - // Return if there is no contentId defined - if (!this.contentId) return; - - 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; - } - - // Ensure the found element is a valid ION-SEGMENT-CONTENT - if (segmentContent.tagName !== 'ION-SEGMENT-CONTENT') { - console.error(`Segment Button: Element with id="${this.contentId}" is not an element.`); - return; - } - // Prevent buttons from being disabled when associated with segment content - if (this.disabled) { + if (this.contentId && this.disabled) { console.warn(`Segment Button: Segment buttons cannot be disabled when associated with an .`); this.disabled = false; } @@ -146,6 +93,24 @@ export class SegmentButton implements ComponentInterface, ButtonInterface { this.inheritedAttributes = { ...inheritAttributes(this.el, ['aria-label']), }; + + // 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}".`); + return; + } + + // Ensure the found element is a valid ION-SEGMENT-CONTENT + if (segmentContent.tagName !== 'ION-SEGMENT-CONTENT') { + console.error(`Segment Button: Element with id="${this.contentId}" is not an element.`); + return; + } } private get hasLabel() { diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 1a3cd15b0d..e2a65407c0 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -388,17 +388,6 @@ 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; -}; - /** * Checks input for usable number. Not NaN and not Infinite. */