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

Auto-generated API docs are no longer displayed automatcally #230

Closed
wants to merge 1 commit into from

Conversation

xcambar
Copy link
Contributor

@xcambar xcambar commented Aug 25, 2018

Displaying the API docs is now opt-in, allowng for better control over the information architcture for the developer

As discussed with @samselikoff, it may be interesting to show the guides and the API reference as 2 top-level entries (regarding the specific use case of ember-cli-mirage).
This change requires that the API guides are not automatically appended to the navbar, as is the case now.

This PR changes the behaviour of the navbar by:

  • yielding a new api-nav component to the docs-viewer/x-nav
  • delegating the API navbar menu to this component
  • Making the display of said docs opt-in to the developer

That requires the developer to do something as follows:

{{#docs-viewer as |viewer|}}

  {{#viewer.nav as |nav|}}
    {{nav.section 'Getting started'}}
    {{nav.item 'Overview' 'docs.getting-started'}}

    {{nav.api-nav}} {{!-- <- This displays the API menu entries --}}
  {{/viewer.nav}}

  {{!-- ... --}}

{{/docs-viewer}}

Displaying the API docs is now opt-in, this allows for better control over the information architcture for the developer
@pzuraq
Copy link
Contributor

pzuraq commented Aug 25, 2018

This is a great PR, I think the idea makes sense given where we’re at. My only concern is about whether we actually want users to have more control over where API docs are placed.

More configurability means more use cases we’ll have to support in the long term. Is it possible we could provide one zero-config solution here instead? Would an API tab be a simpler solution that works for everyone, and if so could we switch to that and not add the ability to optionally have it in the sidebar?

@xcambar
Copy link
Contributor Author

xcambar commented Aug 25, 2018

Here are some food for thoughts:

  • my personal preference is anti-magic, pro-control (not an argument, I know...)
  • the overhead of explicit API docs is little enough to be covered by documentation
  • yet, my intuitive attempt was to actually move the API to its own "context" (route, tab, etc.) just as you suggested, but the amount of work is significantly larger. I thought it would be harder to get accepted for a first contribution without prior consensus.
  • as is, this PR may look like a detour, but we will still be able to achieve the same goal nonetheless. Baby steps.
  • I can think of a backward compatible path in order not to break features:
    • add a "marker" that indicates that api-nav-list has been called in docs-viewer
    • plug to the didRender hook of docs-viewer, check the length of project.modules and the state of the marker, display/warn/deprecate the API docs automatically as necessary.

JSYK, I'll be able (and happy) to follow any implementation path discussed so far. I feel I don't know enough of the project to make a contextually significant opinion though, so I'll just follow the Contributors' lead.

@samselikoff
Copy link
Contributor

samselikoff commented Aug 25, 2018 via email

@pzuraq
Copy link
Contributor

pzuraq commented Aug 26, 2018

@xcambar I really am grateful for this PR (and now the other one)! Honestly, the main reason I'm trying to scope down here is that we don't have that much dev time between Dan, Sam, and I to maintain the project, and the more API space we add, the worse it gets 😬 Totally hear your experience here though, it's a large project to jump into and I think this was an excellent first PR. Glad to have you helping out 😄

From a UX perspective, I think having a top level tab for API docs makes most sense. It mirrors the official Ember guides, and gives us a pretty solid separation of concerns. API docs can be very large (in the case of Ember/EmberData, for instance) and having them be mixed in with the left navbar is tricky.

This is less convenient for smaller addons, but I don't think it's that bad. Even if an addon consists of only one export (function, class, etc) it would still make sense for the user, given it's pretty standard from API doc generation tools. If it were that small, they may actually be able to skip the "guides" portion entirely, and just have a landing page and the API docs tab.

@Kerrick
Copy link

Kerrick commented Jul 23, 2019

For those of us who are excited about this PR for the explicit purpose of getting rid of our auto-generated API docs... is there a way in the current build to opt-out of those?

@samselikoff
Copy link
Contributor

@Kerrick you can use @hide on doc comments to have them not be added...

We'd probably also accept a PR to just move the autogenerated docs to an api-docs yielded variable right here:

https://github.com/ember-learn/ember-cli-addon-docs/blob/master/addon/components/docs-viewer/x-nav/template.hbs#L36

Not a huge deal for consumers, since consumers already have to render a nav. For example Mirage's nav looks like this, and this would just be one extra line at the bottom of something like {{nav.api-docs}}.

@RobbieTheWagner
Copy link
Member

@samselikoff what's the status on this? Do we want to merge or close?

@samselikoff
Copy link
Contributor

I think we should close this one as it's 1.5 years old and if someone's motivated to work on this feature, make the api-docs yielded so consumers can choose to render or not. We should make it backwards-compatible so maybe use a prop.

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.

5 participants