-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
External Documentation rendered for groups, operations and schemata. #595
Conversation
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.
Thanks for the PR.
It looks good.
The one thing that makes me uncomfortable is using <p>
in one place and <div>
in another.
Maybe converting <Link>
from span
to div
and not wrapping it at all will do the trick.
{externalDocs && ( | ||
<Row> | ||
<MiddlePanel> | ||
<p> |
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.
Do we really need a <p>
here?
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.
That's on purpose. The p adds a margin between the External Documentation link and the description. Without the margin in looks odd. The div is chosen in the schema as it seems to be designed to be very small and therefore without margin, like most other elements in there, too. So I think your suggestion makes things look worse without introducing additional styling per place that it's added to.
@@ -50,6 +51,11 @@ export class Operation extends React.Component<OperationProps> { | |||
</H2> | |||
{options.pathInMiddlePanel && <Endpoint operation={operation} inverted={true} />} | |||
{description !== undefined && <Description source={description} />} | |||
{externalDocs && ( | |||
<p> |
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.
Same here
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.
See above.
I cleaned it up a bit myself! Thanks for the PR. It will land in the upcoming alpha at the end of the week. |
Thank you! |
The externalDocs were missing for tags, schemata and operations. Added them with this pull requests. Solves #550.
Additionally, fixes that the URL in External Documentation Object was specified to be optional, which is not correct according to OpenAPI spec.