Skip to content

ControlValueAccessor - #4421

Closed
vsavkin wants to merge 2 commits into
angular:masterfrom
vsavkin:form_value_accessor
Closed

ControlValueAccessor#4421
vsavkin wants to merge 2 commits into
angular:masterfrom
vsavkin:form_value_accessor

Conversation

@vsavkin

@vsavkin vsavkin commented Sep 30, 2015

Copy link
Copy Markdown
Contributor

Closes #4253

@vsavkin vsavkin added action: review The PR is still awaiting reviews from at least one requested reviewer comp: core/forms labels Sep 30, 2015
@vsavkin vsavkin assigned tbosch and rkirov and unassigned tbosch Sep 30, 2015
vicb
vicb reviewed Sep 30, 2015
View reviewed changes
Comment thread modules/angular2/src/core/forms/directives/control_value_accessor.ts Outdated

@vicb vicb Sep 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: "NgValueAccessor" (missing "e")

@vsavkin vsavkin Sep 30, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@vicb

vicb commented Sep 30, 2015

Copy link
Copy Markdown
Contributor

Nice change, LGTM

gkalpak
gkalpak reviewed Sep 30, 2015
View reviewed changes
Comment thread modules/angular2/src/core/forms/directives/ng_control_status.ts

@gkalpak gkalpak Sep 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OOC, would @Self private _cd: NgControl work (instead of setting it "manually") ?

@vsavkin vsavkin Sep 30, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it does not work

@gkalpak

gkalpak commented Sep 30, 2015

Copy link
Copy Markdown
Member

Not directly related to this PR, but do we really need to keep and expose state about mutually exclusive value (e.g. untouched vs touched, pristine vs dirty, invalid vs valid) ?

rkirov
rkirov reviewed Sep 30, 2015
View reviewed changes
Comment thread modules/angular2/src/core/forms/directives/shared.ts Outdated

@rkirov rkirov Sep 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you sure we dont want to throw when null or empty? Is an NgControl without an accessor useful at all (other than testing?)

@vsavkin vsavkin Sep 30, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will throw later. This is to make it a non-breaking change.

@rkirov rkirov added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 30, 2015
@vsavkin
vsavkin force-pushed the form_value_accessor branch from 39ca949 to a3d605c Compare September 30, 2015 22:55
@mary-poppins

mary-poppins commented Sep 30, 2015

Copy link
Copy Markdown

Merging PR #4421 on behalf of @vsavkin to branch presubmit-vsavkin-pr-4421.

@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Sep 30, 2015
@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Sep 30, 2015
@vsavkin
vsavkin force-pushed the form_value_accessor branch from a3d605c to 1f9b18b Compare October 1, 2015 00:52
@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Oct 1, 2015
@mary-poppins

mary-poppins commented Oct 1, 2015

Copy link
Copy Markdown

Merging PR #4421 on behalf of @vsavkin to branch presubmit-vsavkin-pr-4421.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Oct 1, 2015
@vsavkin vsavkin closed this in 00a4b2e Oct 1, 2015
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 7, 2019

Copy link
Copy Markdown

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 Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@rkirov rkirov

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Forms: cyclic DI dependency when component is both valueAccessor and validator

7 participants

@vsavkin @vicb @gkalpak @mary-poppins @rkirov @tbosch @googlebot