Add support for examples in OpenAPI block #2314

vibhanshub · 2024-05-14T15:29:23Z

Same description as #2313

argos-ci · 2024-05-14T15:36:16Z

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	⚠️ Changes detected (Review)	30 changed	May 22, 2024, 10:58 AM

Eloutmanixx

xxxxxxxxxx

SamyPesse · 2024-05-15T09:20:01Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -92,6 +93,12 @@ export function OpenAPISchemaProperty(
                            className="openapi-schema-description"
                        />
                    ) : null}
+                    {schema.example ? (


example is not supposed to be markdown so why use markdown here? (https://swagger.io/docs/specification/adding-examples/)

It can result in problems, ex: if an example contains a markdown string, it'll be processed there when the user expects to see the raw string.

We want to render a value instead of markdown text.

Also the spacing with the description is not great, we should use a gap between the 2

This is what it looks like now

Using margin instead of padding now

Also, argos is complaining for the right reasons as Examples are not present in argos. Can I approve it or do I need to update the images argos is using as a baseline? (first time working with argos where errors are justified)

The font styling looks off to me. Have you checked that?

@taranvohra Hmm, it looks fine to me. I can't see anything in my changes that would affect the font styling.

The text color seems to stand about a bit too much. Is it black (#000)? Is it the same text color as the property name (iso-id in your screenshot)

This is what it looks like with the same color as property name

.openapi-schema-example { @apply mt-2 text-base text-dark/10 dark:text-light/10; }

Looks the same to me

Idk why but this looks a bit better to me haha. It's subtle but it doesn't come across as fully black. Anyway, lgtm otherwise

SamyPesse

IMO it can't be merged in the current state and it requires more work:

Properly support non-primitive values?
- should we represent with JSON.stringify? If yes, it'll not work well for XML/etc API
- should we skip non-primitives using typeof schema.example === 'string' || typeof schema.example === 'number' || typeof schema.example === 'boolean'?
Should we handle examples as well by selecting the first example or the one matching the media-type?

SamyPesse · 2024-05-21T12:11:08Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -14,6 +14,7 @@ interface OpenAPISchemaPropertyEntry {
    propertyName?: string;
    required?: boolean;
    schema: OpenAPIV3.SchemaObject;
+    example?: string;


I don't understand why we need this prop. Isn't example included in the schema prop (https://github.com/kogosoftwarellc/open-api/blob/main/packages/openapi-types/index.ts#L166) ?

Ok it looks like example is for parameters and examples is for schema fields. I think we should try to handle both the same way.

It looks like you don't actually use this

Suggested change

example?: string;

and you use schema.example.

@SamyPesse it looks like you got it reversed. example is for schema fields, and examples is for parameters

https://swagger.io/docs/specification/adding-examples/

I also think we should keep this PR just for the singular example and not support examples for now. What do you think?

Yes, but it doesn't change the fact that this prop is actually not used:

Suggested change

example?: string;

SamyPesse · 2024-05-21T12:14:24Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -92,6 +93,9 @@ export function OpenAPISchemaProperty(
                            className="openapi-schema-description"
                        />
                    ) : null}
+                    {schema.example ? (
+                        <span className="openapi-schema-example">Example: {schema.example} </span>


We should use a <code> or <pre> around the value.

Also, how does it work if my example is not a primitive?

ex:

example: propA: a propB: b

My take is that it'll render [object Object] which is bad.

Are you doing the changes here?

done. this is what it looks like now

SamyPesse · 2024-05-21T12:16:28Z

src/components/DocumentView/OpenAPI/style.css

@@ -172,6 +172,10 @@
    @apply prose-sm;
 }

+.openapi-schema-example {
+    @apply mt-2 text-base text-dark/10 dark:text-light/10;


Why are we using a larger text font than the description?

It doesn't look good in the screenshot.

fixed. see the screenshot above

SamyPesse · 2024-05-21T12:19:45Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -92,6 +93,9 @@ export function OpenAPISchemaProperty(
                            className="openapi-schema-description"
                        />
                    ) : null}
+                    {schema.example ? (


This condition is wrong.

If I have:

type: boolean example: false

It'll not show

(or even empty string, etc).

We should check the typeof and probably skip non-primitives.

Doesn't the same problem exist for schema.description right above this line as well? What if that is false?

I can of course do the typeof check and skip non-primitives

It's about the type. schema.description is always a string and if it's empty we don't want to show an empty div.

schema.example can be any value, depending on the schema.

SamyPesse · 2024-05-21T17:07:04Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -14,6 +14,7 @@ interface OpenAPISchemaPropertyEntry {
    propertyName?: string;
    required?: boolean;
    schema: OpenAPIV3.SchemaObject;
+    example?: string;


Yes, but it doesn't change the fact that this prop is actually not used:

Suggested change

example?: string;

SamyPesse · 2024-05-21T17:09:54Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -92,6 +93,9 @@ export function OpenAPISchemaProperty(
                            className="openapi-schema-description"
                        />
                    ) : null}
+                    {schema.example ? (


It's about the type. schema.description is always a string and if it's empty we don't want to show an empty div.

schema.example can be any value, depending on the schema.

SamyPesse · 2024-05-21T17:10:46Z

packages/react-openapi/src/OpenAPISpec.tsx

@@ -48,6 +48,7 @@ export function OpenAPISpec(props: { rawData: any; context: OpenAPIClientContext
                                // Description of the parameter is defined at the parameter level
                                // we use display it if the schema doesn't override it
                                description: parameter.description,
+                                example: parameter.example,


Why do we need this? Isn't example on the parameter.schema itself.

To maintain compatibility with Swagger 2.0 (in our docs we say we support 2.0). In 2.0, example is not on parameter.schema but on parameter itself.

In 3.0, example is on parameter.schema, so the destructuring happening in the line below will take of it

https://swagger.io/docs/specification/2-0/adding-examples/

SamyPesse · 2024-05-21T17:10:56Z

packages/react-openapi/src/OpenAPISchema.tsx

@@ -92,6 +93,9 @@ export function OpenAPISchemaProperty(
                            className="openapi-schema-description"
                        />
                    ) : null}
+                    {schema.example ? (
+                        <span className="openapi-schema-example">Example: {schema.example} </span>


Are you doing the changes here?

Add support for examples in OpenAPI block

7e31f48

vibhanshub mentioned this pull request May 14, 2024

Add support for examples in the OpenAPI block #2313

Closed

github-actions bot deployed to gitbook-open (Preview) May 14, 2024 15:31 View deployment

Eloutmanixx approved these changes May 14, 2024

View reviewed changes

Fix classname

1345854

github-actions bot deployed to gitbook-open (Preview) May 15, 2024 08:32 View deployment

SamyPesse requested changes May 15, 2024

View reviewed changes

Implement requested changes

af21a15

github-actions bot deployed to gitbook-open (Preview) May 15, 2024 12:22 View deployment

Use margin

ecb9e2c

vibhanshub requested a review from SamyPesse May 15, 2024 12:31

github-actions bot deployed to gitbook-open (Preview) May 15, 2024 12:31 View deployment

scazan approved these changes May 20, 2024

View reviewed changes

taranvohra approved these changes May 21, 2024

View reviewed changes

Use lighter black color

678843c

github-actions bot deployed to gitbook-open (Preview) May 21, 2024 09:59 View deployment

SamyPesse requested changes May 21, 2024

View reviewed changes

SamyPesse reviewed May 21, 2024

View reviewed changes

Review

7dc4b7a

github-actions bot deployed to gitbook-open (Preview) May 22, 2024 10:51 View deployment

Remove extra line

45e34d0

github-actions bot deployed to gitbook-open (Preview) May 22, 2024 10:54 View deployment

Add support for examples in OpenAPI block #2314

Are you sure you want to change the base?

Add support for examples in OpenAPI block #2314

Conversation

vibhanshub commented May 14, 2024 • edited

argos-ci bot commented May 14, 2024 • edited

Eloutmanixx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vibhanshub May 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SamyPesse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vibhanshub commented May 14, 2024 •

edited

argos-ci bot commented May 14, 2024 •

edited

vibhanshub May 15, 2024 •

edited