-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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): cache attached view reference #26694
Conversation
Run & review this pull request in StackBlitz Codeflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a test for this. Also why does this issue not impact Ionic React?
Also be sure to note that this fixes #26695 when merging |
React only uses the framework delegate for I'll confirm #26695 today & fill in a test 👍 Edit: I misunderstood what issue was being linked to. I will reference that issue when merging, I need to also confirm the original intended fix behind this change against the dev-build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go once we have a test. Thanks for taking care of this!
Ok - test case has been added. Also confirmed against the original root issue: https://github.com/raymer/ionic-modal-bug that the expected behavior still occurs after this change. |
Pull request checklist
Please check if your PR fulfills the following requirements:
ionic-docs
repo, in a separate PR. See the contributing guide for details.npm run build
) was run locally and any changes were pushednpm run lint
) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Developers using Vue can run into an error when using an
ion-select
that presents a popover of options. The Vue delegate was incorrectly assuming that a component instance would always be passed as the argument toattachViewToDom
.This is not the case for
ion-select
, where a string of the tag name is used: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/select/select.tsx#L414Issue URL: #26643 (comment)
resolves #26695
What is the new behavior?
WeakMap
always receives an object to that can be used for looking up and unmounting an overlay.any
) or a string. Note: Because any will overwrite the types, this does nothing from a typings perspective. This is just a visual signal to a developer.Does this introduce a breaking change?
Other information
Repro: https://github.com/acoldfox/ionic-select-issue (you will need to install the dev-build to verify)
Dev-build:
6.5.2-dev.11674852369.1c9fe737