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(vue): account for event name changes in vue 3.0.6+ for overlay components #23100

Merged
merged 3 commits into from Mar 29, 2021

Conversation

liamdebeasi
Copy link
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)
  • 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?

See #22980
I forgot to account for the overlay components here as well

What is the new behavior?

  • Events now fire correctly based upon vue version. This will be removed in Framework v6 in favor of the kebab case approach.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the package: vue @ionic/vue package label Mar 24, 2021
@liamdebeasi
Copy link
Contributor Author

liamdebeasi commented Mar 25, 2021

TODO: Write a test that ensures onWillPresent, onDidPresent, onWillDismiss, and onDidDismiss callbacks are fired from an overlay component. DONE

@liamdebeasi liamdebeasi merged commit 27318cf into master Mar 29, 2021
@liamdebeasi liamdebeasi deleted the vue-overlay-event branch March 29, 2021 19:30
/**
* Vue 3.0.6 fixed a bug where events on custom elements
* were always converted to lower case, so "ionRefresh"
* became "ionRefresh". We need to account for the old
Copy link

Choose a reason for hiding this comment

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

@liamdebeasi Shouldn't this be became "ionrefresh" (all lowercase) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks! (#23192)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: vue @ionic/vue package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants