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(angular): keepContentsMounted modal is sized correctly #26917

Merged
merged 5 commits into from
Mar 7, 2023
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Mar 6, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
    • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue URL: resolves #26916

The fix in 00d10f5 uncovered an issue in the angular modal integration where .ion-page was being added twice when keepContentsMounted="true".

The reason is that we add an .ion-page element on mount:

template: `<div class="ion-page" *ngIf="isCmpOpen || keepContentsMounted">
. However, this causes the following condition to be true in our core framework delegate:
} else if (BaseComponent.children.length > 0) {

As a result, .ion-delegate-host and .ion-page are added. However, we then append the children inside of the modal. This results in the .ion-page that Angular created to be added inside another .ion-page.

We have a check to prevent duplicate work by checking ion-delegate-host is present, but the Angular modal never had that class applied.

This issue has been around for quite a while, but we only noticed it in v6.6.0. As a result, I'm not considering this a regression.

What is the new behavior?

  • Added .ion-delegate-host to the element that the Angular modal creates.
  • Added a test.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 6.6.1-dev.11678112836.1733c8ec

@stackblitz
Copy link

stackblitz bot commented Mar 6, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@liamdebeasi liamdebeasi changed the title Fw 3625 fix(angular): keepContentsMounted modal is sized correctly Mar 6, 2023
@github-actions github-actions bot added the package: angular @ionic/angular package label Mar 6, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review March 6, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: angular @ionic/angular package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: ion-datetime-button doesn't open ion-datetime modal
2 participants