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

Use more json schema annotations in the basic form #5532

Merged
merged 4 commits into from
Oct 21, 2022

Conversation

antgamdia
Copy link
Contributor

Description of the change

As pointed out by @jl-beast at #5512, we could leverage more JSON Schema built-in annotations to enrich the UX.

This PR (and the stacked ones) is adding support for examples, readOnly, deprecated, maxItems, multipleOf amongst others (following http://json-schema.org/draft/2020-12/json-schema-validation.html#name-a-vocabulary-for-structural)

Benefits

Richer UX in the basic form for complex schemas

Possible drawbacks

N/A

Applicable issues

Additional information

Note
This PR is part of a series of PRs aimed at closing this milestone. I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes).
PR 3 out of 6

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Conflicts:
	dashboard/src/shared/schema.ts
</div>
</>
</div>
);
}

export function renderConfigType(value: IBasicFormParam) {
return renderCellWithTooltip(value, "type", "italics");
const stringType =
value?.type === "array" ? `${value?.type}<${value?.items?.type}>` : value?.type;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
value?.type === "array" ? `${value?.type}<${value?.items?.type}>` : value?.type;
value?.type === "array" ? `array of ${value?.items?.type}` : value?.type;

Minor suggestion, as it is for human reading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done (in a follow-up PR)

After:
image

Before:
image

Copy link
Collaborator

@castelblanque castelblanque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from 2-5519-yamlDecodingFix to 1-3535-customSchema October 21, 2022 16:31
@antgamdia antgamdia merged commit 291a4f0 into 1-3535-customSchema Oct 21, 2022
@antgamdia antgamdia deleted the 3-5525-schemaAnnotations branch October 21, 2022 16:42
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.

3 participants