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.

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

Enhance documentation UI #182

Merged
merged 45 commits into from
Sep 20, 2019
Merged

Enhance documentation UI #182

merged 45 commits into from
Sep 20, 2019

Conversation

ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Sep 9, 2019

Fixes #172

This changes the HTML/CSS of the OpenAPI documentation using redoc instead of swagger-ui.

Before (screenshot)

before

After (screenshot)

after

Before (URL): https://5d7672277da50b0008d42dda--open-api.netlify.com
After (URL): https://deploy-preview-182--open-api.netlify.com

Improvements over previous setup:

  • prettier
  • use Netlify branding (logo, colors, fonts)
  • mobile-friendlier
  • search bar
  • can add custom documentation sections, for example for Pagination, Rate limiting, etc. Those sections are not added yet.
  • possibility to show different code examples for Go, Node.js, HTTP or other languages/protocols. Note that our specification does not have code examples yet.1
  • much simpler setup. Redoc does everything: bundling, local server, etc. No need for us to maintain that code anymore.

Not changed:

  • support all OpenAPI keywords

To be done:

  • allow sending requests from the browser like in Swagger UI. I'm not sure that one is possible but I can look into it. We should probably recommend importing the OpenAPI definition in Postman or Insomnia instead anyway, as this provides with a better developer experience.
  • add sections for models (as opposed to endpoints)
  • lots of dependencies and code can be removed since redoc does everything. I can do the cleanup after this PR is merged.
  • add favicon

@netlify
Copy link

netlify bot commented Sep 9, 2019

Deploy preview for open-api ready!

Built with commit 60a7f7a

https://deploy-preview-182--open-api.netlify.com

@DavidWells
Copy link
Contributor

This is super awesome 🎉

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 11, 2019

Upgraded the logo (thanks @iKristy!). Looks much better now!
I'll update the favicon after this is merged.

@DavidWells
Copy link
Contributor

One note:

We will need to update netlify api --list command links when we take this live.

image

@verythorough
Copy link
Contributor

Oof, bummer redirects won't work since those are all behind a hash. :(

@DavidWells
Copy link
Contributor

We can handle redirects client side.

if hash && match('#/default/') > redirect to new url

#/default/createSite to #operation/createSite

redoc/build.js Outdated
const redocCli = async function() {
await pExecFile('redoc-cli', [
`--title=${TITLE}`,
`--options.expandResponses=${SUCCESS_STATUS_CODES}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on disabling this?

When 200 response are auto expanded by default it makes scrolling much much longer.

I tried the build with this line commented out and it was easier to quickly scroll down the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 275ecc4

@ehmicky ehmicky mentioned this pull request Sep 18, 2019
@ehmicky ehmicky force-pushed the feature/enhance-ui branch 2 times, most recently from 6182ee5 to 81cff8f Compare September 18, 2019 15:59
@ehmicky ehmicky mentioned this pull request Sep 18, 2019
@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 18, 2019

I've added the following:

  • favicons
  • <meta> tags (copied from netlify-www)
  • sections copied from the online documentation: "Making a request", "Authenticating", "Rate limiting", "Pagination", "Site IDs"

The endpoints are also now grouped by models. The models themselves are grouped when they are related.

When it comes to the links (that you mentioned above), I've opened netlify/cli#546 to update the links of netlify api --list.

I have removed all the code related to the previous setup (based on Swagger UI).

Finally I made the following refactoring changes:

  • split into smaller files
  • moved all JavaScript-related code to src/ and API documentation to src/docs/, so it's easier to distinguish form the Go-related code at go/
  • separate the injected JavaScript to its own script.js and the injected HTML to its own head.html so it's easier to add more to them

I need some feedback on the following:

  • I named the endpoint groups ("tags" and "tag groups") based on my intuition. However since I don't know the API I might have gotten them wrong. Could you please look at the left sidebar and make sure the grouping and the group names make sense?
  • Is there anything left to do when it comes to the links and redirects?
  • I would need an additional code review for this big PR, thanks @DavidWells and @verythorough! :)

@ehmicky ehmicky requested review from DavidWells and verythorough and removed request for futuregerald September 18, 2019 18:04
Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

sections copied from the online documentation: "Making a request", "Authenticating", "Rate limiting", "Pagination", "Site IDs"

Please remove these. Keeping identical docs in two places in an anti-pattern and will lead to drift. The copied text is already out of sync with an updated version of the text currently in a PR for docs 2.0 migration.

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 19, 2019

@verythorough fixed!

Is there anything left because this can be deployed to production?

Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the cli links with netlify/cli#546!

Could you add the JS redirect as @DavidWells suggested? That way links that we can't account for (in blog posts, etc.) will still work, too.

Otherwise, I think the tags/tag groups you chose work fine, and this is a nice improvement on what was there before. Looking forward to working on our next iteration!

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 20, 2019

Oh I did not understand @DavidWells comment on the redirects, I get it now. Very good point!

I've fixed it in 60a7f7a

You can try for example https://deploy-preview-182--open-api.netlify.com/#/default/listSites

Ready for another round :)

Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

Awesome! Works great for me! 🚀

@ehmicky ehmicky merged commit 8e543ca into master Sep 20, 2019
@ehmicky ehmicky deleted the feature/enhance-ui branch September 20, 2019 18:00
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.

Enhance the HTML template
3 participants