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.

Sign up for GitHub

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

Jump to bottom

build(aio): do not render "constructor()" heading in API docs #22380

Closed
wants to merge 2 commits into from
Closed

build(aio): do not render "constructor()" heading in API docs #22380

wants to merge 2 commits into from

Conversation

petebacondarwin
Copy link
Member

Closes #22363

@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer comp: aio labels Feb 22, 2018
@petebacondarwin petebacondarwin added this to REVIEW in docs-infra Feb 22, 2018
@mary-poppins
Copy link

You can preview 5faca5a at https://pr22380-5faca5a.ngbuilds.io/.

@petebacondarwin
Copy link
Member Author

Before:

screen shot 2018-02-22 at 21 07 50

After:

screen shot 2018-02-22 at 21 07 20

@petebacondarwin petebacondarwin mentioned this pull request Feb 22, 2018
Closed
gkalpak
gkalpak approved these changes Feb 23, 2018
View reviewed changes
aio/tools/transforms/templates/api/lib/memberHelpers.html Outdated
@@ -93,7 +93,7 @@ <h4 class="no-anchor">Throws</h4>
{%- macro renderMethodDetails(methods, containerClass, itemClass, headingText) -%}
{% if methods.length %}
<section class="{$ containerClass $}">
<h2>{$ headingText $}</h2>
{% if headingText %}<h2>{$ headingText $}</h2>{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

Are we using this macro without headingText? This change doesn't seem related to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, sorry that slipped through from my playing around.

@gkalpak gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 23, 2018
petebacondarwin
petebacondarwin commented Feb 23, 2018
View reviewed changes
aio/tools/transforms/templates/api/class.template.html Outdated
{% if doc.constructorDoc %}{$ memberHelpers.renderMethodDetail(doc.constructorDoc, 'constructor') $}{% endif %}
{% if doc.constructorDoc %}
<h2>Constructor</h2>
{$ memberHelpers.renderMethodDetail(doc.constructorDoc) $}{% endif %}
Copy link
Member Author

Choose a reason for hiding this comment

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

We should still pass the cssClass as the second parameter too.

@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 23, 2018
@mary-poppins
Copy link

You can preview 87aa27b at https://pr22380-87aa27b.ngbuilds.io/.

@petebacondarwin petebacondarwin added the target: major This PR is targeted for the next major release label Feb 24, 2018
@mary-poppins
Copy link

You can preview f29c759 at https://pr22380-f29c759.ngbuilds.io/.

@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 25, 2018
@petebacondarwin petebacondarwin moved this from REVIEW to MERGE in docs-infra Feb 25, 2018
@mary-poppins
Copy link

You can preview 846e3af at https://pr22380-846e3af.ngbuilds.io/.

@alexeagle alexeagle closed this in 4bd3e5f Feb 27, 2018
@petebacondarwin petebacondarwin removed this from MERGE in docs-infra Feb 28, 2018
@petebacondarwin petebacondarwin deleted the aio-api-constructor-heading branch February 28, 2018 10:33
smdunn pushed a commit to smdunn/angular that referenced this pull request Feb 28, 2018
@petebacondarwin @smdunn809
build(aio): do not render "constructor()" heading in API docs (angula…
287a11f 
…r#22380)

Closes angular#22363

PR Close angular#22380
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
@petebacondarwin @leo6104
build(aio): do not render "constructor()" heading in API docs (angula…
97ebbd8 
…r#22380)

Closes angular#22363

PR Close angular#22380
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gkalpak gkalpak gkalpak approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

aio + docs/api: constructor heading should match Properties and Methods headings
4 participants
@petebacondarwin @mary-poppins @gkalpak @googlebot