-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
improve doc for filter CP #14605
improve doc for filter CP #14605
Conversation
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.
Maybe we can give both examples? I think the example that exists now is probably the more common scenario so we should just give both...
@rwjblue amended |
@@ -278,6 +278,28 @@ export function mapBy(dependentKey, propertyKey) { | |||
hamster.get('remainingChores'); // [{name: 'write more unit tests', done: false}] | |||
``` | |||
|
|||
You can also use `@each` in your dependent key, the callback will still use the underlying array: |
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.
🤔 I'm a bit concerned people will hurriedly read and start using chores.@each
(for example) without the leaf property. Maybe @each.property
? Maybe it's not a concern.
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.
@locks right, amended |
{ name: 'write more unit tests', done: true } | ||
] | ||
}); | ||
hamster.set('done', false); |
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.
What is this setting? should the example be to set the last item of chores
as done: false
? or perhaps just remove the set
example and define one of the chores to have done: false
in the call to Hamster.create
@Leooo hi, if you need help addressing @pixelhandler's comments let us know! Let's get this merged :D |
@locks didn't see @pixelhandler 's comment, on it thanks |
@locks @pixelhandler better? I tested the last example, thanks for spotting the issue |
]) | ||
}); | ||
hamster.get('remainingChores'); // [{name: 'write more unit tests', done: false}] | ||
hamster.get('chores')[2].set('done', true); |
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.
Maybe change this for hamster.get('chores').objectAt(2).set('done', true);
?
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.
@locks done thanks
@Leooo thank you for your work! :) Can you squash the commits for us, and I'll get rwjblue to re-review your PR? |
Thank you! |
Was really not obvious to me before that the
dependentKey
for afilter
CP could be anything different than the array used to filter.