Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Render links to external docs #7559

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Render links to external docs #7559

merged 2 commits into from
Aug 17, 2022

Conversation

fabsrc
Copy link
Contributor

@fabsrc fabsrc commented Oct 15, 2021 •
edited
Loading

Description

Renders links to external docs in all locations where external docs can be used:

  • Tag Object (modified for consistency)
  • Schema Object
    • Object (added)
    • Array (added)
    • Primitive (added)
  • OpenAPI Object / Swagger Object (not changed)
  • Operation (not changed)

Motivation and Context

Fixes #1545

How Has This Been Tested?

TODO

Screenshots (if appropriate):

Bildschirmfoto 2021-10-15 um 18 18 03

Bildschirmfoto 2021-10-15 um 18 18 33

Bildschirmfoto 2021-10-15 um 18 20 10

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

AndriyTykhan
AndriyTykhan reviewed Oct 17, 2021
View reviewed changes
src/core/components/array-model.jsx Outdated
}
}
{
!externalDocsUrl ? null :
Copy link

Choose a reason for hiding this comment

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

Suggested change
!externalDocsUrl ? null :
externalDocsUrl &&

AndriyTykhan
AndriyTykhan reviewed Oct 17, 2021
View reviewed changes
src/core/components/object-model.jsx Outdated
@@ -93,6 +96,17 @@ export default class ObjectModel extends Component {
</td>
</tr>
}
{
!externalDocsUrl ? null :
Copy link

Choose a reason for hiding this comment

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

Suggested change
!externalDocsUrl ? null :
externalDocsUrl &&

AndriyTykhan
AndriyTykhan reviewed Oct 17, 2021
View reviewed changes
src/core/components/operation-tag.jsx Outdated
@@ -88,21 +88,17 @@ export default class OperationTag extends React.Component {
</small>
}

{tagExternalDocsUrl ?
Copy link

Choose a reason for hiding this comment

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

Suggested change
{tagExternalDocsUrl ?
{tagExternalDocsUrl &&

AndriyTykhan
AndriyTykhan reviewed Oct 17, 2021
View reviewed changes
src/core/components/operation-tag.jsx Outdated
target="_blank"
>{tagExternalDocsDescription || "External docs"}</Link>
</small>
</div> : null
Copy link

Choose a reason for hiding this comment

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

Suggested change
</div> : null
</div>

AndriyTykhan
AndriyTykhan reviewed Oct 17, 2021
View reviewed changes
src/core/components/primitive-model.jsx Outdated
@@ -53,6 +56,12 @@ export default class Primitive extends Component {
!description ? null :
<Markdown source={ description } />
}
{
!externalDocsUrl ? null :
Copy link

Choose a reason for hiding this comment

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

Suggested change
!externalDocsUrl ? null :
externalDocsUrl &&

@tim-lai tim-lai self-assigned this Mar 10, 2022
@tim-lai
Copy link
Contributor

tim-lai commented Mar 21, 2022

@fabsrc Thanks for the PR! First, to complete this PR, please add Cypress E2E tests to validate that External docs appears in the expected operations, and that the text doesn't appear where it shouldn't. Also, I am curious to know if you've tried some responsive testing, i.e. narrowing and widening of the viewport, and if there's any CSS impact.

@tim-lai tim-lai mentioned this pull request May 2, 2022
Merged
17 tasks
@fabsrc fabsrc force-pushed the ft/1545-render-external-docs branch 2 times, most recently from fbdb6dd to 4b5fb8f Compare August 12, 2022 17:17
@fabsrc fabsrc force-pushed the ft/1545-render-external-docs branch from 4b5fb8f to 2b2374d Compare August 14, 2022 08:50
@fabsrc
Copy link
Contributor Author

fabsrc commented Aug 14, 2022

@tim-lai I updated the PR and added some Cypress E2E tests. Also checked the responsiveness, which seems to be fine. Could you please have another look?

@tim-lai tim-lai merged commit 6ae2693 into swagger-api:master Aug 17, 2022
@tim-lai
Copy link
Contributor

tim-lai commented Aug 17, 2022

@fabsrc PR merged! Thanks for the contribution and followup!

@fabsrc fabsrc deleted the ft/1545-render-external-docs branch September 5, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndriyTykhan AndriyTykhan AndriyTykhan left review comments

Assignees

@tim-lai tim-lai

Labels
type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Render externalDocs in all applicable locations
3 participants
@fabsrc @tim-lai @AndriyTykhan