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

issue/910: added requireCompletionOf: X #951

Merged
merged 3 commits into from
Mar 7, 2016
Merged

issue/910: added requireCompletionOf: X #951

merged 3 commits into from
Mar 7, 2016

Conversation

oliverfoster
Copy link
Member

#910

if you need all of the mandatory components to be complete use _requireCompletionOf: -1 (this is as default)
if you need to exclude some of the mandatory components from completion use _isOptional: true (making them non-mandatory)
if you need only X number of mandatory components to complete, use requireCompletionOf: X

if you need all of the mandatory components to be complete use _requireCompletionOf: -1  (this is as default)
if you need to exclude some of the mandatory components from completion use _isOptional: true (making them non-mandatory)
if you need only X number of mandatory components to complete, use requireCompletionOf: X
use of === is a faster operator
the default path is represented first
@oliverfoster oliverfoster changed the title issue/910: added requireCompletionOf: "any" issue/910: added requireCompletionOf: X Feb 18, 2016
@LukaszGrela
Copy link
Contributor

+1

@moloko
Copy link
Contributor

moloko commented Feb 18, 2016

+1
This does all need documenting somewhere of course

One thing to note is that setting _requireCompletionOf to 0 triggers much the the same behaviour as setting _isOptional on all components in the block - except that the PLP will not show those components as being optional. I don't think this is problem, so long as we're clear that it does this - perhaps we should recommend in the docs that _isOptional is the better method for making all components in a block optional?

@oliverfoster
Copy link
Member Author

might be better to make if "0" : false / off ? that way we remove the ambiguity? or introduce more?

@taylortom
Copy link
Member

+1
Don't think it particularly matters if it's the default? Otherwise, I'd maybe prefer a value like "all" to a number.

moloko added a commit that referenced this pull request Mar 7, 2016
issue/910: added requireCompletionOf: X
@moloko moloko merged commit a9ab8c7 into master Mar 7, 2016
@moloko moloko deleted the issue/910 branch March 7, 2016 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants