Skip to content

refactor(material/form-field): allow AbstractControlDirective as MatFormFieldControl.ngControl #18206

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

Merged

Conversation

crisbeto
Copy link
Member

Allows for an AbstractControlDirective to be passed in as the MatFormFieldControl.ngControl. The advantage of passing in the abstract control directive is that it allows both form controls and form groups. This is necessary for the date range picker.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Jan 17, 2020
@crisbeto crisbeto requested a review from mmalerba as a code owner January 17, 2020 18:42
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 17, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

Looks like there's some build error

@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from 286d3b5 to 38b62fb Compare January 17, 2020 18:46
@crisbeto crisbeto requested a review from devversion as a code owner January 17, 2020 18:46
@crisbeto
Copy link
Member Author

The build error should be resolved now. I forgot to update the MDC text field.

@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch 2 times, most recently from f4c62f6 to ac8eae8 Compare January 17, 2020 18:55
@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Jan 17, 2020
@jelbourn jelbourn added presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged and removed action: merge The PR is ready for merge by the caretaker labels Jan 22, 2020
@jelbourn
Copy link
Member

@crisbeto this looks like a breaking change. ngControl is a public API, and so we start to see errors like

error TS2322: Type 'AbstractControlDirective | null' is not assignable to type 'NgControl | null | undefined'.
  Type 'AbstractControlDirective' is missing the following properties from type 'NgControl': name, valueAccessor, validator, asyncValidator, viewToModelUpdate

22     this.field = formControl.ngControl;

@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from ac8eae8 to 692dfd0 Compare January 22, 2020 19:30
@crisbeto
Copy link
Member Author

@jelbourn I've changed the type to be less break-ey. Can we do another run?

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Jan 22, 2020
@mmalerba
Copy link
Contributor

I suspect that its just going to complain that AbstractControlDirective | NgControl | null is not assignable to NgControl | null | undefined now

@jelbourn
Copy link
Member

Yep

error TS2322: Type 'NgControl | AbstractControlDirective | null' is not assignable to type 'NgControl | null | undefined'.
  Type 'AbstractControlDirective' is missing the following properties from type 'NgControl': name, valueAccessor, validator, asyncValidator, viewToModelUpdate

22     this.field = formControl.ngControl;
       ~~~~~~~~~~

@jelbourn jelbourn added the target: major This PR is targeted for the next major release label Jan 30, 2020
@jelbourn jelbourn added this to the 10.0.0 milestone Jan 30, 2020
@jelbourn jelbourn removed the target: patch This PR is targeted for the next patch release label Jan 30, 2020
@jelbourn jelbourn modified the milestones: 10.0.0, 11.0.0 Jun 2, 2020
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from 692dfd0 to bff7b4b Compare July 4, 2020 16:33
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from bff7b4b to 924edd4 Compare August 18, 2020 19:51
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from 924edd4 to 4a9b5f8 Compare January 7, 2021 13:17
@crisbeto crisbeto changed the title refactor(form-field): allow AbstractControlDirective as MatFormFieldControl.ngControl refactor(material/form-field): allow AbstractControlDirective as MatFormFieldControl.ngControl Jan 7, 2021
@mmalerba mmalerba removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jan 7, 2021
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from 4a9b5f8 to f22c92a Compare June 29, 2021 18:42
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from f22c92a to 0271043 Compare September 2, 2021 18:35
@crisbeto crisbeto requested a review from a team as a code owner September 2, 2021 18:35
@josephperrott josephperrott removed the request for review from a team September 2, 2021 19:22
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 28, 2021
…ormFieldControl.ngControl

Allows for an `AbstractControlDirective` to be passed in as the `MatFormFieldControl.ngControl`. The advantage of passing in the abstract control directive is that it allows both form controls and form groups. This is necessary for the date range picker.
@crisbeto crisbeto force-pushed the form-field-control-abstract-directive branch from 0271043 to 8145d89 Compare March 22, 2022 13:09
@crisbeto crisbeto merged commit 088f0a8 into angular:master Mar 23, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Apr 3, 2022
…ormFieldControl.ngControl (angular#18206)

Allows for an `AbstractControlDirective` to be passed in as the `MatFormFieldControl.ngControl`. The advantage of passing in the abstract control directive is that it allows both form controls and form groups. This is necessary for the date range picker.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants