Skip to content

Conversation

@mallachari
Copy link

@mallachari mallachari commented Apr 8, 2021

Fixes: stoplightio/json-schema-viewer#122
Needs: stoplightio/json-schema-viewer#132 for jsv divider light color.

Batch of ui fixes (not so mega in the end).

image

@mallachari mallachari requested a review from philsturgeon as a code owner April 8, 2021 22:55
@mallachari mallachari requested a review from a team April 8, 2021 22:55
@mallachari mallachari self-assigned this Apr 8, 2021
@netlify
Copy link

netlify bot commented Apr 8, 2021

Deploy preview for stoplight-elements ready!

Built with commit 60cd548

https://deploy-preview-1033--stoplight-elements.netlify.app

Copy link
Contributor

@mpodlasin mpodlasin left a comment

Choose a reason for hiding this comment

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

Ummm...

I don't think that' intentional? (schemas don't have titles at all).

Screenshot from 2021-04-09 14-22-38

}: ISchemaViewerProps) => {
const [selectedIndex, setSelectedIndex] = React.useState(0);

export const SchemaViewer = ({ className, title, description, schema, errors, viewMode }: ISchemaViewerProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this name.

What is the difference between JsonSchemaViewer and SchemaViewer? 😂

I would go for something like SchemaWithDescription? DescriptionAndSchema?

Copy link
Author

Choose a reason for hiding this comment

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

I used SchemaViewer as there was ISchemaViewerProps interface.
SchemaWithDescription might be fine as it was SchemaWithExamples before.

@mallachari
Copy link
Author

Ummm...

I don't think that' intentional? (schemas don't have titles at all).

Screenshot from 2021-04-09 14-22-38

What do you mean? What's wring with this schema?

@mpodlasin
Copy link
Contributor

Ummm...
I don't think that' intentional? (schemas don't have titles at all).
Screenshot from 2021-04-09 14-22-38

What do you mean? What's wring with this schema?

That's that whole page. Does this look good? Just a blank page with a schema? No title? 🤔

Screenshot from 2021-04-09 14-34-25

@mallachari
Copy link
Author

Should there be a title? There's no title in schema, we could just show a name like in toc but there wasn't anything before and that wasn't the requirement so I didn't touch it.

@mpodlasin
Copy link
Contributor

Should there be a title? There's no title in schema, we could just show a name like in toc but there wasn't anything before and that wasn't the requirement so I didn't touch it.

You are right. This is not a regression. I thought it was. No problem then!

Copy link
Contributor

@mpodlasin mpodlasin left a comment

Choose a reason for hiding this comment

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

Approving, but I am strongly encouraging to change that SchemaViewer name ;)

@mallachari mallachari merged commit b1a1514 into main Apr 9, 2021
@mallachari mallachari deleted the fix/mega-batch branch April 9, 2021 14:13
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.

Mega Batch of JSV UI/UX fixes

3 participants