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

Notify of _subController property change #3312

Closed
wants to merge 1 commit into from
Closed

Notify of _subController property change #3312

wants to merge 1 commit into from

Conversation

bcardarella
Copy link
Contributor

When ArrayControllers instantiate an itemController and insert into the
_subControllers array the member it replaces is undefined. This fix
will push a notification for property change when the actual
itemController replaces the undefined item.

When ArrayControllers instantiate an itemController and insert into the
_subControllers array the member it replaces is `undefined`. This fix
will push a notification for property change when the actual
itemController replaces the `undefined` item.
@bcardarella
Copy link
Contributor Author

I was not sure how best to test this one out, open to suggestions so a regression test is in there.

@wycats
Copy link
Member

wycats commented Sep 3, 2013

In general, we don't notify _ properties. Could you explain the use-case a bit more?

@bcardarella
Copy link
Contributor Author

ember-validations is being rewritten to observe isValid flag on all children. The && of all isValid flags in the validator graph for an object will be the value of the object's isValid. Validation rules can be defined in the models themselves but I am recommending that the rules be defined in the controller. For this issue if the validations are set in the itemControllers then when those itemControllers instantiate the parent controller is not being notified of that and the validation graph is reporting false values as the observer for those items is firing when the undefined is inserted in the subController array.

@bcardarella
Copy link
Contributor Author

@wycats the other option is to add the instance of the itemController directly to _subControllers instead of the undefined placeholder.

@ghost ghost assigned wycats Sep 10, 2013
@wagenet
Copy link
Member

wagenet commented Sep 10, 2013

@wycats, thoughts?

@wagenet
Copy link
Member

wagenet commented Oct 13, 2013

@wycats ping

@bcardarella
Copy link
Contributor Author

Bueller?

@hjdivad
Copy link
Member

hjdivad commented Mar 6, 2014

@bcardarella do you happen to have a jsbin that illustrates this problem?

@bcardarella
Copy link
Contributor Author

Here are the tests and workaround I did for ember-validations: DavyJonesLocker/ember-validations@00a775e

@hjdivad
Copy link
Member

hjdivad commented Mar 6, 2014

see DavyJonesLocker/ember-validations#102.

I believe the tl;dr for ember here is that users should, whenever possible, ignore the implementation detail that an array controller happens to be presenting items behind item controllers, and instead treat the array controller as any other kind of array for the purposes of observers, CPs &c.

So if, for example, one wants to observe changes to arrays, add observers to the [] property.

@bcardarella
Copy link
Contributor Author

Closing. @hjdivad your suggestion is better, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants