8000 Add JSON schema form visual improvements by gusevda · Pull Request #4040 · giantswarm/happa · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add JSON schema form visual improvements #4040

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

Merged
merged 3 commits into from
Jan 26, 2023
Merged

Conversation

gusevda
Copy link
Contributor
@gusevda gusevda commented Jan 26, 2023

What does this PR do?

Visual improvements for hierarchical object and array fields according to the design specs.

How does it look like?

Screenshot 2023-01-26 at 14 15 53

Any background context you can provide?

Towards giantswarm/roadmap#1181.

@gusevda gusevda requested a review from a team January 26, 2023 11:16
@gusevda gusevda force-pushed the json-schema-form-visual-changes branch from 7f591aa to 6a78b98 Compare January 26, 2023 11:19
@marians
Copy link
Member
marians commented Jan 26, 2023

It would be nice to have an example in our test schema where array items are objects with a title. Like this:

                "arrayOfObjectsWithTitle": {
                    "items": {
                        "properties": {
                            "age": {
                                "type": "number"
                            },
                            "name": {
                                "type": "string"
                            }
                        },
                        "type": "object",
                        "title": "Array item object",
                        "description": "Description of array item object"
                    },
                    "title": "Array of objects with title",
                    "type": "array"
                },

@marians
Copy link
Member
marians commented Jan 26, 2023

Why is there an asterisk on the generated object title arrayOfObjects-0 here?

image

@gusevda
Copy link
Contributor Author
gusevda commented Jan 26, 2023

Why is there an asterisk on the generated object title arrayOfObjects-0 here?

The library mark array items as required for some reason. I would need to check what we can do about it.

@gusevda
Copy link
Contributor Author
gusevda commented Jan 26, 2023

It would be nice to have an example in our test schema where array items are objects with a title.

I added an example into the test schema. When an array item has custom title, it's being used without index postfix. If we still want to have index added, I can look into it. It's better to do it in a separate ticket, I think.

children,
}) => {
return (
<ArrayFieldItemW direction='row'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a small suggestion - could we have a more descriptive name than ArrayFieldItemW ? I assumed the W stands for wrapper, but I wasn't sure if the abbreviation was intentional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for noticing, it's definitely unintentional. Will fix it.

Copy link
Contributor
@kuosandys kuosandys left a comment

Choose a reason for hiding this comment

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

Works well for me 👍 My comment above about the naming shouldn't be blocking

Copy link
Member
@marians marians left a comment

Choose a reason for hiding this comment

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

< 8000 select name="classifier" class="form-select mr-2" aria-label="Reason" required>

Good improvement as it is!

@gusevda gusevda merged commit 9a0ebf7 into main Jan 26, 2023
@gusevda gusevda deleted the json-schema-form-visual-changes branch January 26, 2023 13:59
@marians
Copy link
Member
marians commented Jan 26, 2023

Referencing the original issue: giantswarm/roadmap#1181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0