two-step binding form for providing binding parameters #389
Conversation
| Cancel | ||
| </button> | ||
| {displayNameForm ? ( | ||
| <SchemaForm schema={this.nameSchema()} onSubmit={this.handleNameChange}> |
There was a problem hiding this comment.
I would use just Form here instead of SchemaForm it's a bit confusing that you are using the SchemaForm just to retrieve a name.
There was a problem hiding this comment.
I see what you mean but my actual secret plan is to move all forms in Kubeapps over to using the SchemaForm eventually. The main reasoning is that it gives us one place to define how all forms look inside Kubeapps, and avoids boilerplating out <form>s everywhere. That way, the components only need to worry about the schema/fields that go into a form and not the rendering of the form itself. WDYT? If you think that's not a useful thing to have though, we can consider putting this back to a regular form.
There was a problem hiding this comment.
mm, I am not sure if that is something doable? I may be missing something but forms like DeploymentForm are more complex and define methods like componentWillReceiveProps and have nested elements like DeploymentBinding. Anyway, I understood schema like something specific to service bindings but if it's not I am fine reusing it every time is possible.
| @@ -205,7 +210,7 @@ export class InstanceView extends React.Component<IInstanceViewProps> { | |||
| </CardGrid> | |||
| <h2>Bindings</h2> | |||
| <AddBindingButton | |||
| bindingName={instance.metadata.name + "-binding"} | |||
| bindingSchema={svcPlan && svcPlan.spec.serviceBindingCreateParameterSchema} | |||
There was a problem hiding this comment.
not sure if this change is related to the issue?
There was a problem hiding this comment.
perhaps you're misunderstanding the issue in #363, @arapulido wasn't able to create the bindings for the GCP PubSub service because there were required parameters and previously this form didn't support specifying parameters for creating bindings (we only supported them for provisioning instances). So this PR is to add support for the schema provided by the broker to render a form for entering binding parameters.
There was a problem hiding this comment.
I see now, I didn't realized how pieces are tied together
|
@andresmgot the fix for #363 is to add |
|
👍 I guess you want to implement the |
|
Thanks! Actually I'll do it in a separate PR. |
refs #363