Allow disabled choices #197
Conversation
Signed-off-by: Tom Kremer <[email protected]>
lib/widgets.js
Outdated
| var renderData = isNested ? | ||
| { isNested: true, label: choice[0], choices: choice[1] } : | ||
| { isNested: false, value: choice[0], label: choice[1] }; | ||
| if (choice.length == 3) { |
There was a problem hiding this comment.
i'm not sure we need this branching - we can just set disabled to false unless a third truthy value is provided.
- Accept a 4th param for the default option - Update Test Signed-off-by: Tom Kremer <[email protected]>
| @@ -57,8 +57,8 @@ var renderChoices = function (choices, renderer) { | |||
| return reduce(choices, function (partialRendered, choice) { | |||
| var isNested = is.array(choice[1]); | |||
| var renderData = isNested ? | |||
| { isNested: true, label: choice[0], choices: choice[1] } : | |||
| { isNested: false, value: choice[0], label: choice[1] }; | |||
| { isNested: true, label: choice[0], choices: choice[1], disabled: choice[2], default: choice[3] } : | |||
There was a problem hiding this comment.
what is "default"? let's keep this PR just to adding "disabled".
|
i gotta comment even though I’m not a maintainer but it’s my code you’re changing :). i like the idea, but apart from the fact, that this change only provides an array syntax for specifying disabled options, i always feel like options that use designated array indexes are hard to remember and force developers to look at the docs a lot because they forgot which option is which array-index. the wish to omit the disable or default flag (this pr introduces both as of now) and the need to have special values to skip an array index is another problem i see with this implementation. why not keep the current format as it is and add defaults and disabled values as individual field options? iirc values are unique for each field and you could just build object where choice values map to default values/ disabled flags. that’d be backwards compatible, easy to remember and you probably would’t have to touch any code beside the choice renderer. btw. how do you see choice defaults working out? i might be mistaken but I don’t see the point because you already have full control over the values as the user simply selects given choices. |
|
I haven't truly yet understood this PR, but I agree that anything that requires remembering or matching array indices is just not a good API. |
Set the third value in choices to TRUE to disable an item on the list.