Skip to content

Conversation

@OS-pedrolourenco
Copy link
Contributor

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
  • No

Other information

@vercel
Copy link

vercel bot commented Jan 14, 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 Jan 15, 2025 0:15am

@github-actions github-actions bot added the package: core @ionic/core package label Jan 14, 2025
@OS-pedrolourenco OS-pedrolourenco marked this pull request as ready for review January 14, 2025 16:26
@OS-pedrolourenco OS-pedrolourenco requested a review from a team as a code owner January 14, 2025 16:26
Copy link
Contributor

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Some minor comments

let animationFrameId: number;

const check = () => {
const segmentView = this.getNextSiblingOfType<HTMLIonSegmentViewElement>(ionSegment!); // Skip the text nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving the if (!ionSegment) { into the check method instead so we can avoid using !?

reject(new Error(`Segment not found when looking for Segment Content`));
}

let timeoutId: any = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's properly type this

Suggested change
let timeoutId: any = null;
let timeoutId: number | undefined = undefined;

Copy link
Contributor

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@OS-giulianasilva OS-giulianasilva left a comment

Choose a reason for hiding this comment

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

Nice!

@OS-pedrolourenco OS-pedrolourenco merged commit fe0b32f into next Jan 16, 2025
47 checks passed
@OS-pedrolourenco OS-pedrolourenco deleted the ROU-11523 branch January 16, 2025 16:04
OS-pedrolourenco added a commit that referenced this pull request Jan 17, 2025
…nt has not yet been created (#30133)

Issue number: internal

---------

<!-- 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. -->
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?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- `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

<!--
  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. -->
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.

4 participants