-
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
feat(input): ionChange will only emit from user committed changes #25858
Conversation
angular/src/directives/control-value-accessors/numeric-value-accessor.ts
Outdated
Show resolved
Hide resolved
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.
The changes look good, though I am not sure I understand the purpose of the value accessor updates.
angular/src/directives/control-value-accessors/numeric-value-accessor.ts
Show resolved
Hide resolved
angular/src/directives/control-value-accessors/numeric-value-accessor.ts
Outdated
Show resolved
Hide resolved
General note surrounding the value accessors. I compared against using To match this pattern with the change in behavior to |
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.
Great job!
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
<!-- Please refer to our contributing documentation for any questions on submitting a pull request, or let us know here if you need any help: https://ionicframework.com/docs/building/contributing --> <!-- Some docs updates need to be made in the `ionic-docs` repo, in a separate PR. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation for details. --> <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> <!-- Issues are required for both bug fixes and features. --> Unbuilt changes in `core` are not always being caught because the `api.txt` file is not cached. Stencil will potentially update both the `src/components.d.ts` and `api.txt` files when there are built changes to the public API. We need to check both files when accounting for unbuilt changes. Example: #25858 This had updates to `api.txt` that were not built. Built changes PR: #25933 Example of this change catching `api.txt` changes as intended: #27165 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - GitHub Actions core build step caches `api.txt` ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
<!-- Please refer to our contributing documentation for any questions on submitting a pull request, or let us know here if you need any help: https://ionicframework.com/docs/building/contributing --> <!-- Some docs updates need to be made in the `ionic-docs` repo, in a separate PR. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation for details. --> <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> <!-- Issues are required for both bug fixes and features. --> Unbuilt changes in `core` are not always being caught because the `api.txt` file is not cached. Stencil will potentially update both the `src/components.d.ts` and `api.txt` files when there are built changes to the public API. We need to check both files when accounting for unbuilt changes. Example: #25858 This had updates to `api.txt` that were not built. Built changes PR: #25933 Example of this change catching `api.txt` changes as intended: #27165 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - GitHub Actions core build step caches `api.txt` ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
<!-- Please refer to our contributing documentation for any questions on submitting a pull request, or let us know here if you need any help: https://ionicframework.com/docs/building/contributing --> <!-- Some docs updates need to be made in the `ionic-docs` repo, in a separate PR. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation for details. --> <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> <!-- Issues are required for both bug fixes and features. --> Unbuilt changes in `core` are not always being caught because the `api.txt` file is not cached. Stencil will potentially update both the `src/components.d.ts` and `api.txt` files when there are built changes to the public API. We need to check both files when accounting for unbuilt changes. Example: #25858 This had updates to `api.txt` that were not built. Built changes PR: #25933 Example of this change catching `api.txt` changes as intended: #27165 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - GitHub Actions core build step caches `api.txt` ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the 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?
ionChange
is emitted from both internal and external value changes ofion-input
.Angular
ion-input
form control value updates whenionChange
is emitted (ngModel
,formControl
,formControlName
).Issue URL: Resolves #20106, #20061
What is the new behavior?
ionChange
will only be emitted from user committed changes toion-input
:ionChange
will not be emitted from externally assigning a value toion-input
Angular
ion-input
form control value updates whenionInput
is emitted (ngModel
,formControl
,formControlName
).Does this introduce a breaking change?
Developers that need the immediate value of the input as the user types should migrate their implementations to using
ionInput
instead.Developers that assign a value directly to
ion-input
should not expectionChange
to be emitted. In these cases, developers should execute expected side-effect behavior when assigning the value.Other information
This is the first component being resolved of this issue. As others are migrated, we will likely have a "Form controls" section in the
BREAKING
doc, instead of copy/pasting for each affected component.Once all of the components of a specific value accessor are migrated, the temporary value accessors will be removed and the legacy class name will be the same API.
Dev-build:
6.2.4-dev.11661977223.1d42c906