Skip to content

Commit 1cfa915

Browse files
thetaPCthetaPC
authored
fix(segment-button): ensure consistent disabled state for segment-content error handling (#30288)
Issue number: N/A --------- <!-- 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? <!-- Please describe the current behavior that you are modifying. --> 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](#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? <!-- Please describe the behavior or changes that are being added by this PR. --> - 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 <!-- 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. --> 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".`
1 parent 7789bb5 commit 1cfa915

File tree

2 files changed

+21
-67
lines changed

2 files changed

+21
-67
lines changed

‎core/src/components/segment-button/segment-button.tsx

+21-56
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { ComponentInterface } from '@stencil/core';
22
import { Component, Element, Host, Prop, Method, State, Watch, forceUpdate, h } from '@stencil/core';
33
import type { ButtonInterface } from '@utils/element-interface';
44
import type { Attributes } from '@utils/helpers';
5-
import { addEventListener, removeEventListener, inheritAttributes, getNextSiblingOfType } from '@utils/helpers';
5+
import { addEventListener, removeEventListener, inheritAttributes } from '@utils/helpers';
66
import { hostContext } from '@utils/theme';
77

88
import { getIonMode } from '../../global/ionic-global';
@@ -65,69 +65,16 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
6565
this.updateState();
6666
}
6767

68-
private waitForSegmentContent(ionSegment: HTMLIonSegmentElement | null, contentId: string): Promise<HTMLElement> {
69-
return new Promise((resolve, reject) => {
70-
let timeoutId: NodeJS.Timeout | undefined = undefined;
71-
let animationFrameId: number;
72-
73-
const check = () => {
74-
if (!ionSegment) {
75-
reject(new Error(`Segment not found when looking for Segment Content`));
76-
return;
77-
}
78-
79-
const segmentView = getNextSiblingOfType<HTMLIonSegmentViewElement>(ionSegment); // Skip the text nodes
80-
const segmentContent = segmentView?.querySelector(
81-
`ion-segment-content[id="${contentId}"]`
82-
) as HTMLIonSegmentContentElement | null;
83-
if (segmentContent && timeoutId) {
84-
clearTimeout(timeoutId); // Clear the timeout if the segmentContent is found
85-
cancelAnimationFrame(animationFrameId);
86-
resolve(segmentContent);
87-
} else {
88-
animationFrameId = requestAnimationFrame(check); // Keep checking on the next animation frame
89-
}
90-
};
91-
92-
check();
93-
94-
// Set a timeout to reject the promise
95-
timeoutId = setTimeout(() => {
96-
cancelAnimationFrame(animationFrameId);
97-
reject(new Error(`Unable to find Segment Content with id="${contentId} within 1000 ms`));
98-
}, 1000);
99-
});
100-
}
101-
102-
async connectedCallback() {
68+
connectedCallback() {
10369
const segmentEl = (this.segmentEl = this.el.closest('ion-segment'));
10470
if (segmentEl) {
10571
this.updateState();
10672
addEventListener(segmentEl, 'ionSelect', this.updateState);
10773
addEventListener(segmentEl, 'ionStyle', this.updateStyle);
10874
}
10975

110-
// Return if there is no contentId defined
111-
if (!this.contentId) return;
112-
113-
let segmentContent;
114-
try {
115-
// Attempt to find the Segment Content by its contentId
116-
segmentContent = await this.waitForSegmentContent(segmentEl, this.contentId);
117-
} catch (error) {
118-
// If no associated Segment Content exists, log an error and return
119-
console.error('Segment Button: ', (error as Error).message);
120-
return;
121-
}
122-
123-
// Ensure the found element is a valid ION-SEGMENT-CONTENT
124-
if (segmentContent.tagName !== 'ION-SEGMENT-CONTENT') {
125-
console.error(`Segment Button: Element with id="${this.contentId}" is not an <ion-segment-content> element.`);
126-
return;
127-
}
128-
12976
// Prevent buttons from being disabled when associated with segment content
130-
if (this.disabled) {
77+
if (this.contentId && this.disabled) {
13178
console.warn(`Segment Button: Segment buttons cannot be disabled when associated with an <ion-segment-content>.`);
13279
this.disabled = false;
13380
}
@@ -146,6 +93,24 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
14693
this.inheritedAttributes = {
14794
...inheritAttributes(this.el, ['aria-label']),
14895
};
96+
97+
// Return if there is no contentId defined
98+
if (!this.contentId) return;
99+
100+
// Attempt to find the Segment Content by its contentId
101+
const segmentContent = document.getElementById(this.contentId) as HTMLIonSegmentContentElement | null;
102+
103+
// If no associated Segment Content exists, log an error and return
104+
if (!segmentContent) {
105+
console.error(`Segment Button: Unable to find Segment Content with id="${this.contentId}".`);
106+
return;
107+
}
108+
109+
// Ensure the found element is a valid ION-SEGMENT-CONTENT
110+
if (segmentContent.tagName !== 'ION-SEGMENT-CONTENT') {
111+
console.error(`Segment Button: Element with id="${this.contentId}" is not an <ion-segment-content> element.`);
112+
return;
113+
}
149114
}
150115

151116
private get hasLabel() {

‎core/src/utils/helpers.ts

-11
Original file line numberDiff line numberDiff line change
@@ -388,17 +388,6 @@ export const shallowEqualStringMap = (
388388
return true;
389389
};
390390

391-
export const getNextSiblingOfType = <T extends Element>(element: Element): T | null => {
392-
let sibling = element.nextSibling;
393-
while (sibling) {
394-
if (sibling.nodeType === Node.ELEMENT_NODE && (sibling as T) !== null) {
395-
return sibling as T;
396-
}
397-
sibling = sibling.nextSibling;
398-
}
399-
return null;
400-
};
401-
402391
/**
403392
* Checks input for usable number. Not NaN and not Infinite.
404393
*/

0 commit comments

Comments
 (0)