Skip to content

fix(angular): keepContentsMounted modal is sized correctly - #26917

Merged
liamdebeasi merged 5 commits into
mainfrom
FW-3625
Mar 7, 2023
Merged

fix(angular): keepContentsMounted modal is sized correctly#26917
liamdebeasi merged 5 commits into
mainfrom
FW-3625

Conversation

@liamdebeasi

@liamdebeasi liamdebeasi commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

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

@bolt-new-by-stackblitz

Copy link
Copy Markdown

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
@liamdebeasi
liamdebeasi merged commit 8d184c8 into main Mar 7, 2023
@liamdebeasi
liamdebeasi deleted the FW-3625 branch March 7, 2023 14:25
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