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

Adapt "basic forms" to a JSONSchema-compliant spec #4917

Closed
antgamdia opened this issue Jun 14, 2022 · 5 comments
Closed

Adapt "basic forms" to a JSONSchema-compliant spec #4917

antgamdia opened this issue Jun 14, 2022 · 5 comments
Assignees
Labels
component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature

Comments

@antgamdia
Copy link
Contributor

antgamdia commented Jun 14, 2022

Summary
Currently, the "basic form" feature is using a custom format to represent the data that should be rendered (pics below). This format is not standard nor it is being properly updated by the Bitnami team.
Besides, the current implementation becomes unusable when the number of params in the form is more than ~10 ?

Background and rationale

If Bitnami (the primary producer of content with this schema format) stops creating this file and moves towards a usual JSONSchema (see bitnami/readme-generator-for-helm#15), the "basic form" functionality will be useless.

Description

First, we should define which x-whicheverCustomFormProperty we are going to use (bitnami/readme-generator-for-helm#15).
Once agreed, we have to implement a UI form somehow. For instance, using jsonschema-forms (as in https://github.com/vmware-tanzu/kubeapps/pull/3527/files) or a tabular mode (as suggested in #4396).

Or even we can have both: the forms-based approach for the "basic form" and the tabular one for the "advanced view" or something like that.

Doing so would solve, IMO both #2540 and #1924.

Additionally, we could revisit #3535 to see how we can include custom schemas coming from 3rd party places.

Acceptance criteria

  • We have designed an alternative for rendering the forms, preserving (when possible) the current functionality.
  • There is a coordinated plan with Bitnami for them to stop publishing the current schema and start serving the new one.
  • We have implemented the "basic form" alternative.

Additional context

image
image

@antgamdia antgamdia added component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature next-iteration Issues to be discussed in planning session labels Jun 14, 2022
@antgamdia antgamdia added this to the Form refactor milestone Jun 14, 2022
@ppbaena ppbaena removed the next-iteration Issues to be discussed in planning session label Aug 1, 2022
@antgamdia antgamdia self-assigned this Aug 18, 2022
@antgamdia
Copy link
Contributor Author

antgamdia commented Aug 18, 2022

Some random thoughts before digging into this issue:

  • If we happen to replace today's basic forms with a new solution, all the historical series of released charts would be incompatible with this new schema-based form.
    • We can support both, however, we would be maintaining old and potentially unnecessary code.
    • This is a kind of a blind decision as we don't have data for actually deciding with certainty.
    • I'd opt for dropping support unless we got a request explicitly asking for it.
  • As discussed offline, the schema generator tool is not gonna add new x-xxxxx params, so we lose this succinct and opinionated selection of "basic" params.
    • In a step 2, we are going to need a way to defer to the users/admins this "basic" params extraction. IMO, this would be a separate issue that would require a proper design doc. This issue, as I see it, should just be scoped to "the bare minimum changes that allow bitnami generating the jsonschema-compliant file that we are able to consume it"
  • There are several ideas on how to present the info back to the users, from a table to an actual HTML-alike form. As an initial step in solving this issue, we should ensure the technical feasibility of each option. I'll focus on this part near soon.
  • The schema URL is hardcoded, see https://github.com/antgamdia/kubeapps/blob/942e4d6a81ded59d03c4bfbf4f3568e0ebe0ff3f/pkg/tarutil/tarutil.go#L74

@antgamdia
Copy link
Contributor Author

Some (very) initial results of the table-mode view:

Image

Still in a PoC-phase, I'm looking for better solutions for rendering the tables (with built-in filters, search, etc...) as well as getting the information from both the schema + appValues and looking for a solution for keeping everything in sync easily.
Keep you posted.

@castelblanque
Copy link
Collaborator

That looks promising! Shouldn't we track the task in #4396 ?

@antgamdia
Copy link
Contributor Author

Quick update here:

We have designed an alternative for rendering the forms, preserving (when possible) the current functionality.

We finally went with the tabular mode, as it was the best approach for rendering large schemas without compromising the UI performance. Tracked at: #4396

We have implemented the "basic form" alternative.

It is mainly implemented at #5412

There is a coordinated plan with Bitnami for them to stop publishing the current schema and start serving the new one.

Bitnami folks have been informed, but any change on their side will have to wait until we release a version including the changes in #5412.

Next steps: merge the changes, create an issue with the pending items, revisit every issue related to the old "basicforms" approach and check with our adopters how this change would affect them.

@antgamdia
Copy link
Contributor Author

antgamdia commented Oct 6, 2022

After the merge of the ongoing PRs, some TODOs still remain. I've created some issues for tracking each of them:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature
Projects
Archived in project
Development

No branches or pull requests

3 participants