-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Improve autogenerated API docs #14549
Conversation
### setup | ||
|
||
[src/index.js#L16-L26](src/index.js#L16-L26) | ||
<a name="setup" href="#setup">#</a> **setup** [\<>](src/index.js#L16-L26) |
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.
Maybe not to be addressed here, but the relative links for code are dead links in NPM:
https://www.npmjs.com/package/@wordpress/a11y
It appears NPM tries to append it to the GitHub repository, and it may be that once they properly support the new repository.directory
on their website it becomes fixed.
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.
Curious, was there some prior art for the <>
? Is it meant to be an abbreviated message of "this is the code!" ? Once I understood it, it seemed fine enough. But I'd be lying if I said my initial reaction wasn't to consider it as being some erroneous output.
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.
Yeah, there is an issue for the links. I plan to address it next. It's also a problem in the handbook.
The <>
idea I took it from D3 API docs such as: https://github.com/d3/d3-brush/blob/master/README.md#api-reference
### setup | ||
|
||
[src/index.js#L16-L26](src/index.js#L16-L26) | ||
<a name="setup" href="#setup">#</a> **setup** [\<>](src/index.js#L16-L26) |
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.
I'd be curious if NPM would strip anything of the custom anchor tag. Is there a reason it ought not be a heading, or is that the idea of "toning down"? Structurally / semantically, it seemed to make sense as a heading.
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.
Not really, just that this approach made the symbols more prominent. I'm on mobile now but can upload some before/after images.
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.
Since it's auto-generated it probably doesn't matter that much whether we use heading. However, I'm curious if we could keep h3 as it helped to give more focus to the method name.
### setup | ||
|
||
[src/index.js#L16-L26](src/index.js#L16-L26) | ||
<a name="setup" href="#setup">#</a> **setup** [\<>](src/index.js#L16-L26) |
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.
Were you still planning to apply the code formatting, as per #14290 ?
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.
Tried that as well but liked this more.
10c2b2c
to
797917f
Compare
When the symbol receives a params object and the keys are documented, we want those names to be shown.
797917f
to
c4cadf2
Compare
Rebased and formatted the recently added data package. |
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.
It's fine as is, I would be fine to make method names stand out more. Let's get this in and iterate :)
We definitely need to address relative URLs separately. It's way more important.
Closes #14290
It makes the symbol names to stand out and be more compact by toning down other parts of the symbol description.