-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
build(aio): jsdoc tag changes #22401
build(aio): jsdoc tag changes #22401
Conversation
You can preview 4a9a812 at https://pr22401-4a9a812.ngbuilds.io/. |
Here is a good example of the The rendered docs look like: While the original source code comment looks like: |
You can preview 4c56f8b at https://pr22401-4c56f8b.ngbuilds.io/. |
4c56f8b
to
b4d9007
Compare
You can preview b4d9007 at https://pr22401-b4d9007.ngbuilds.io/. |
b4d9007
to
9d42efc
Compare
You can preview 9d42efc at https://pr22401-9d42efc.ngbuilds.io/. |
You can preview a138ceb at https://pr22401-a138ceb.ngbuilds.io/. |
expect(docs[2].description).toEqual('the description'); | ||
}); | ||
|
||
it('should ignore docs that have neither `howToUseIt` nor `whatItDoes` properties', () => { |
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.
howToUseIt --> howToUse 😁
margin: 24px 0px 10px; | ||
background: $lightgray; | ||
clear: both; | ||
} |
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.
This change seems totally unrelated to this commit. Maybe move it in a separate one 😇
Also, there is one more h1:after
style in _typography.scss
and one in _marketing-layout.scss
that are redundant now.
@@ -1,5 +0,0 @@ | |||
{%- if doc.whatItDoes %} | |||
<div class="what-it-does info-banner"> |
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.
If we don't use info-banner
any more here, could we remove the corresponding CSS styles?
@@ -1,10 +1,10 @@ | |||
{% extends 'base.template.html' -%} | |||
|
|||
{% block body %} | |||
{% include "includes/what-it-does.html" %} | |||
<p class="short-description">{$ doc.shortDescription | marked $}</p> |
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.
Is it possible to not have shortDescription
? In that case, would it be better to omit the <p>
altogether?
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.
@@ -4,7 +4,7 @@ | |||
{% extends 'base.template.html' -%} | |||
|
|||
{% block body %} | |||
<p>{$ doc.whatItDoes | marked $}</p> | |||
<p class="short-description">{$ doc.shortDescription | marked $}</p> |
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.
Is it possible to not have shortDescription
? In that case, would it be better to omit the <p>
altogether?
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 think that the short description will come under the "required" parts of the API docs.
@@ -0,0 +1,5 @@ | |||
module.exports = function(log, createDocMessage) { |
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.
Unused deps 😁
docs.forEach(doc => { | ||
if (this.docTypes.indexOf(doc.docType) !== -1 && doc.description !== undefined) { | ||
const description = doc.description.trim(); | ||
const endOfParagraph = description.indexOf('\n\n'); |
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.
Is it possible to have whitespace between the two \n
?
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.
That would be nasty 😠 but OK, I guess we can handle that too.
const processorFactory = require('./splitDescription'); | ||
const Dgeni = require('dgeni'); | ||
|
||
fdescribe('splitDescription processor', () => { |
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.
Oops
* The first paragraph is now split off into the `shortDescription` property. * Usage of `howToUse` and `whatItDoes` have been updated. * The "Overview" heading for class is removed as it is self-evident * The original horizontal rule styling below the main heading is removed as not part of the new design Closes angular#22385
… and `@howToUse`
… and `@howToUse`
… and `@howToUse`
a0a37e2
to
6d0e882
Compare
You can preview 5bd18b0 at https://pr22401-5bd18b0.ngbuilds.io/. |
You can preview 6d0e882 at https://pr22401-6d0e882.ngbuilds.io/. |
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.
💯
@@ -60,13 +50,7 @@ h6 { | |||
clear: both; | |||
} | |||
|
|||
h1 { | |||
@media screen and (max-width: 600px) { | |||
margin: 0; |
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.
Why remove this style?
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.
Previously there was an underline beneath h1
tags. This underline was given the appropriate margin (8px 0
) for the surrounding text. Now that the underline has gone the margins are set by the following rule.
That being said, I now see that this rule also removes margin from before the h1
on narrow screens so I will put that back in.
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.
See last fixup commit 1443bfb
You can preview 1443bfb at https://pr22401-1443bfb.ngbuilds.io/. |
…howToUse` (#22401) See #22135 (comment) PR Close #22401
* The first paragraph is now split off into the `shortDescription` property. * Usage of `howToUse` and `whatItDoes` have been updated. * The "Overview" heading for class is removed as it is self-evident * The original horizontal rule styling below the main heading is removed as not part of the new design Closes #22385 PR Close #22401
* The first paragraph is now split off into the `shortDescription` property. * Usage of `howToUse` and `whatItDoes` have been updated. * The "Overview" heading for class is removed as it is self-evident * The original horizontal rule styling below the main heading is removed as not part of the new design Closes angular#22385 PR Close angular#22401
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Addresses #22135 and #22385