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

Changed documentation so method signature and body stay together for overloaded functions #51

Merged
merged 4 commits into from
May 3, 2017

Conversation

mray190
Copy link
Contributor

@mray190 mray190 commented May 2, 2017

The old documentation had method signatures (function definition) lumped together, and then displayed method bodies (comments, description, example, parameters, returns) together, for overloaded functions. This was confusing as you didn't know where one body finished and the next one started. Additionally didn't have the method signature directly next to the body.

This PR separates out each method so that each overloaded function has a signature and body inline. New change shown below:

image

Note: The way the templates are setup, this will lose the blue icon color next to each function. Not sure how to fix it... If you think the blue icon is mandatory to go public, I will invest more time into it.

Additionally, I think it would be optimal for the description to occur first under the function header, and then show the parameters/return values for each overloaded function after that with an example. Proposed result shown below:

image

I will attempt to do this last way in a different pull request

…ead of listing all signatures first, and then all the bodies
@mray190
Copy link
Contributor Author

mray190 commented May 2, 2017

@thegecko
Copy link
Contributor

thegecko commented May 2, 2017

This is the default template, I'm not sure of this is used to generate the final docs or not. @micque01 Can you confirm?

@mray19027 I'd also recommend you consider a PR to the main repo over at https://github.com/TypeStrong/typedoc-default-themes

@thegecko
Copy link
Contributor

thegecko commented May 2, 2017

@iriark01 Which style do you prefer?

@iriark01
Copy link
Contributor

iriark01 commented May 3, 2017

I like having the description as the first thing you read. Also like having the example come last, after you've been given all the information the example works with.

@micque01
Copy link

micque01 commented May 3, 2017

@thegecko I've taken these templates to use on the cloud docs site, but they are in the codebase at: https://github.com/ARMmbed/cloud-web/tree/master/cloud/docs/static/typedoc/theme, so the templates in this repo are not used to generate the docs on the cloud site

@@ -1,6 +1,6 @@
<ul class="tsd-parameters">
{{#if signatures}}
<li class="tsd-parameter-siganture">
Copy link
Contributor

Choose a reason for hiding this comment

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

This is mis-spelt everywhere else, too. Fixing the spelling here breaks the CSS

@thegecko
Copy link
Contributor

thegecko commented May 3, 2017

@mray19027 I've updated this PR to remove the spelling fix as the CSS has it mis-spelt too. If it bothers you a lot, feel free to open a PR to fix all instances.
I've also fixed the blue icon issue using the hbs paramter path.
With regards to a single description, you could show the first signature description for each method, but as each method signature can have it's own description, you may end up hiding information. Therefore I recommend we don't change it.

Finally, could you re-create this PR for https://github.com/ARMmbed/cloud-web, too

@thegecko thegecko merged commit a50efe4 into PelionIoT:master May 3, 2017
@mray190
Copy link
Contributor Author

mray190 commented May 3, 2017

@thegecko Thanks for fixing that blue icon!
Didn't realize that spelling issue was an issue across multiple files. Glad you caught that and left it as is.
I also had submitted a pull request on the theme's github page yesterday. It is located here: TypeStrong/typedoc-default-themes#49

I will re-create this PR for https://github.com/ARMmbed/cloud-web as well

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.

None yet

4 participants