feat(@angular/cli): adding checkbox prompt support #13004
Conversation
* Added general checkbox prompt support * Added checkbox prompt for guard schematic implementation
|
Created new PR because I messed up the commit subject format on the first one. This way too the history is cleaner. |
There was a problem hiding this comment.
Thank you for the contribution.
Can you split this into two PRs? The core prompt changes and the schematic changes are two separate features.
Also, there should be separate commits for each package modified. In this case, a feat(@angular/cli) and a feat(@angular-devkit/core for core prompt improvements within this one PR. And a separate PR for the schematic changes with a feat(@schematics/angular).
Thank you again.
| @@ -309,7 +308,8 @@ export abstract class SchematicCommand< | |||
| question.type = 'confirm'; | |||
| break; | |||
| case 'list': | |||
| question.type = 'list'; | |||
| case 'checkbox': | |||
| question.type = definition.type; | |||
There was a problem hiding this comment.
The initial plan for this concept was to model this as a multiselect question option on the list type itself. Could you make that change?
| 'one', | ||
| 'two', | ||
| 'three', | ||
| ], |
There was a problem hiding this comment.
This is an invalid schema, something cannot be an array and also be equal to one/two/three. For array definition, the items field should be used. For additional information, please see here: https://json-schema.org/understanding-json-schema/reference/array.html
| @@ -109,7 +109,7 @@ export interface PromptDefinition { | |||
| } | |||
|
|
|||
| export type PromptProvider = (definitions: Array<PromptDefinition>) | |||
| => SubscribableOrPromise<{ [id: string]: JsonValue }>; | |||
| => SubscribableOrPromise<{ [id: string]: JsonValue } | { [id: string]: JsonValue }[]>; | |||
There was a problem hiding this comment.
This shouldn't need to be changed. Each id (i.e., field) should have one value (a JsonValue can be an array).
There was a problem hiding this comment.
That makes sense. This was a bit of an artifact from trying to resolve some typing errors I had caused while debugging. Good catch!
|
@clydin I would be happy to get those done ASAP! Thank you for the review/information. |
|
Closing to split features into two PRs. Please refer to #13031 fpr checkbox/multiselect support. Guard schematic changes to come. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Hi there!
This is my first contribution, and actually my first contribution to an open source project of this size. Please let me know if there is anything I may have missed in terms of adding tests or could have done better etc.
Very excited to submit this and hope the community will find it useful!
Thanks,
Michael
PR description:
Added general checkbox support using inquirer for schematics prompts
Added checkbox 'implements' option to guard schematics to allow user to choose implementations (CanActivate, CanActivateChild or CanLoad)
Notes:
I Purposefully left out CanDeactivate because of the added complexity of needed a Component to implement. I was definitely considering that as a follow up to this PR, but since that would take a little more time on my part I wanted to get this out there first.