-
Notifications
You must be signed in to change notification settings - Fork 182
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
feat: add support for vendor extension renderers #2545
feat: add support for vendor extension renderers #2545
Conversation
@@ -23,7 +24,7 @@ export const isBodyEmpty = (body?: BodyProps['body']) => { | |||
return contents.length === 0 && !description?.trim(); | |||
}; | |||
|
|||
export const Body = ({ body, onChange }: BodyProps) => { | |||
export const Body = ({ body, onChange, renderExtensionAddon }: BodyProps) => { | |||
const [refResolver, maxRefDepth] = useSchemaInlineRefResolver(); | |||
const [chosenContent, setChosenContent] = React.useState(0); | |||
const { nodeHasChanged } = useOptionsCtx(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, I am pulling through the renderExtensionAddon
-property in each relevant component. I was wondering if I should get it from useOptionsCtx()
-hook instead?
Please advise what your expected or preferred approach is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is great! We are super excited about this.
We would prefer that you add it to the useOptionsCtx()
Additionally could you create a elements type to replace the import { ExtensionAddonRenderer } from '@stoplight/json-schema-viewer';
so we are not dependent on the json-schema-viewer implementation? Also please add tests and storybook example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @brendarearden. I hope to pick up end of next week and make the requested changes.
I also would like to introduce the functionality to expand/collapse all properties of a schema. Only I don't know yet what the best approach to achieve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you would be able to utilize the expandedDepth
from the SchemaTreeOptions
in json-schema-viewer to achieve that functionality. If you do decide to introduce that, it would be preferable to have a separate PR from this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely in a new PR for that one. I think the main issue I saw that the expansion state is stored in RowSchemaRow
-component but probably better to discuss the json-schema-viewer
-repo first. Probably worth to add the expandAll/collapseAll
-functionality there first.
Going to pick up this PR soon. A few work stuff that is more important but after that I am going to do some Elements related work so expect work on this PR and maybe this expandAll/collapseAll
-functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @weyert are you planning on continuing this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next week 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-white Am I blocking you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-white @brendarearden Would be fine to import SchemaNode
from the @stoplight/json-schema-tree
-package?
✅ Deploy Preview for stoplight-elements-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for stoplight-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
# Conflicts: # packages/elements-dev-portal/package.json # packages/elements/package.json
@daniel-white @brendarearden I have updated the PR based on your feedback |
"type": { | ||
"type": "string", | ||
"enum": ["STANDARD", "ADMIN"], | ||
"x-enum-descriptions": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated a bunch of fixtures, like this, one to accommodate for testing vendor extensions functionality both in Storybook and unit tests
@@ -16,6 +18,10 @@ const exampleSchema: JSONSchema7 = { | |||
propA: { | |||
type: 'string', | |||
enum: ['valueA'], | |||
// @ts-ignore | |||
'x-enum-descriptions': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason JSONSchema7
don't support vendor extensions. It's probably a OpenAPI enhancement for JSON. schema?
I am not sure if that's expected but that is the reason for the @ts-ignore
.
} | ||
|
||
// This implementation of the extension renderer only supports the `x-enum-descriptions`-extension | ||
if ('x-enum-descriptions' in vendorExtensions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a simple implementation of a vendor extension
@@ -40,6 +41,8 @@ interface ISchemaAndDescriptionProps { | |||
|
|||
const SchemaAndDescription = ({ title: titleProp, schema }: ISchemaAndDescriptionProps) => { | |||
const [resolveRef, maxRefDepth] = useSchemaInlineRefResolver(); | |||
const { renderExtensionAddon } = useOptionsCtx(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to allow schemas referred in the Article
-component also work with the vendor extension renderers
tryItCredentialsPolicy={tryItCredentialsPolicy} | ||
tryItCorsProxy={tryItCorsProxy} | ||
/> | ||
<ElementsOptionsProvider renderExtensionAddon={renderExtensionAddon}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason the API*
components don't see to wrap around the ElementsOptionsProvider
. This is required to be able to use useOptionsCtx
in elements-core
.
I appreciate some guidance if this the right approach you had in mind.
Changed the handling of vendor extensions to allow to render vendor extensions in the body of a `Model` or `HttpOperation` between the title and the request block
@brendarearden @daniel-white I am assigned this week to do work related to Elements. Would love to get some feedback on this PR. |
@weyert due to the size of this PR, we will have to dedicate specific time in our work flow to review this. We have added an issue for this work to our backlog and will prioritize it. |
Okay, then I will try to make a small enhancement to it that we would like to have internally. We think it would be useful to also have access to the schema-level vendor extensions when rendering a property of the schema and then always call the renderer when at least one of the two are not empty. As we would like to render a marking when the property is expandable but define it on the schema level instead of On another topic I would love to be able to email with someone about the allOf merging behaviour of Elements or |
# Conflicts: # packages/elements-core/package.json # packages/elements-core/src/components/Docs/HttpOperation/HttpOperation.stories.ts # packages/elements-dev-portal/src/version.ts # yarn.lock
50dcec9
to
24261da
Compare
b610235
to
43582b3
Compare
43582b3
to
aabc69d
Compare
Thank you for merging this one :D |
Adds support for defining vendor extension renderers in
@stoplight/elements
so vendor extensions throughout a OpenAPI specification can be visually renderer.Elements Default PR Template
In general, make sure you have: (check the boxes to acknowledge you've followed this template)
CONTRIBUTING.md