Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(segment-button): ensure consistent disabled state for segment-content error handling #30288

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Mar 20, 2025

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. 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
  • 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 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
  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".

Copy link

vercel bot commented Mar 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2025 8:28pm

@github-actions github-actions bot added the package: core @ionic/core package label Mar 20, 2025
@thetaPC thetaPC changed the title refactor(segment-button): update when the errors are displayed refactor(segment-button): update error handling for segment-content Mar 20, 2025
@thetaPC thetaPC marked this pull request as ready for review March 20, 2025 23:50
@thetaPC thetaPC requested a review from a team as a code owner March 20, 2025 23:50
@thetaPC thetaPC requested review from brandyscarney and ShaneK March 20, 2025 23:50
Copy link
Member

@ShaneK ShaneK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Seems like a solid solution 🚀

@brandyscarney
Copy link
Member

Shouldn't this be a fix since it fixes the disabled states?

@thetaPC thetaPC changed the title refactor(segment-button): update error handling for segment-content fix(segment-button): update error handling for segment-content Mar 25, 2025
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@thetaPC thetaPC changed the title fix(segment-button): update error handling for segment-content fix(segment-button): ensure consistent disabled state for segment-content error handling Mar 25, 2025
@thetaPC thetaPC added this pull request to the merge queue Mar 25, 2025
Merged via the queue into main with commit 1cfa915 Mar 25, 2025
53 checks passed
@thetaPC thetaPC deleted the segment-errors branch March 25, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants