Skip to content

Conversation

sean-perkins
Copy link
Contributor

Issue number: Internal


What is the current behavior?

The GestureController provider does not use the correct underlying instance of the utilities from either the lazy or custom elements build, depending on if the developer is using @ionic/angular or @ionic/angular/standalone. It will always use the lazy instance.

This applied to the createGesture function.

What is the new behavior?

  • GestureController uses the instance of the utilities based on it's implementation type, e.g. @ionic/angular/standalone uses the custom elements build with the utilities from @ionic/core/components.
  • @ionic/angular and @ionic/angular/standalone now export their own specific implementation of GestureController

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the package: angular @ionic/angular package label Nov 6, 2023
@sean-perkins sean-perkins marked this pull request as ready for review November 6, 2023 22:24
@sean-perkins sean-perkins requested a review from thetaPC as a code owner November 6, 2023 22:24
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

Should we add tests?

@sean-perkins
Copy link
Contributor Author

@thetaPC which tests would you suggest here?

The custom code that is specific to the Angular implementation is: https://github.com/ionic-team/ionic-framework/pull/28477/files#diff-bed6d361bb9f4f8db3c7f80b9ded377fc71817a99a84f1172b40a4c497bbf1e4R14-R21, but would be better suited with an unit test vs. e2e test - validating that gesture callbacks are ran through a zone. We do not currently have any test infrastructure for unit tests in the angular test apps.

@thetaPC
Copy link
Contributor

thetaPC commented Nov 9, 2023

@sean-perkins if we can setup the test apps with unit tests in the future then we should add the test that you suggested. For now, we can not leave it as is.

Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@sean-perkins sean-perkins added this pull request to the merge queue Nov 10, 2023
Merged via the queue into main with commit f0a5d27 Nov 10, 2023
@sean-perkins sean-perkins deleted the sp/FW-5485 branch November 10, 2023 02:16
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.

2 participants