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

fix(docs-infra): do not include GitHub links in Table of Content #32418

Closed
wants to merge 2 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Aug 30, 2019

The docs template for cli commands (cli-command.template.html) includes an h2 element with GitHub links for long description. Since the content of h2 elements is replicated in the auto-generated Table of Contents, the GitHub links were replicated as well (which is undesirable).

This commit fixes it by explicitly excluding .github-links elements, when extracting the content for the ToC (in TocService#extractHeadingSafeHtml()). This is similar to what we do for the auto-generated .header-link anchors.

@gkalpak gkalpak added type: bug/fix action: merge The PR is ready for merge by the caretaker comp: docs-infra target: patch This PR is targeted for the next patch release labels Aug 30, 2019
@gkalpak gkalpak added this to the docs-infra-aio-app milestone Aug 30, 2019
@gkalpak gkalpak requested a review from a team as a code owner August 30, 2019 14:18
console.error(
'\nCheck CONTRIBUTING.md at the root of the repo for more information.' +
'\n' +
'\n(In case you need the invalid commit message, it should be stored in \'.git/COMMIT_EDITMSG\'.)');
Copy link
Member

Choose a reason for hiding this comment

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

LOL did you get bitten by this?

Copy link
Member Author

Choose a reason for hiding this comment

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

More times than I want to admit 😇

(I thought I had removed this change from this PR. I submitted #32420.)

@mary-poppins
Copy link

You can preview 129dee6 at https://pr32418-129dee6.ngbuilds.io/.

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Actually there seem to be a bunch of legitimate failures.

@mary-poppins
Copy link

You can preview cab10e0 at https://pr32418-cab10e0.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 48fabd5 at https://pr32418-48fabd5.ngbuilds.io/.

@mary-poppins
Copy link

You can preview dea05af at https://pr32418-dea05af.ngbuilds.io/.

@gkalpak
Copy link
Member Author

gkalpak commented Aug 30, 2019

It turns out there was another issue with tooltips (title attribute) of ToC items that the tests didn't hit before. I added another commit to fix tha too.

@petebacondarwin: PTAL 👀

@petebacondarwin petebacondarwin added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 30, 2019
The Table of Contents (ToC) is auto-generated based on the content of
heading elements on the page. At the same time, anchor links are
auto-generated and added to each heading element. Note that the Material
Icons used for the anchor icon make use of ligatures, which means that
the icons are specified by using their textual name as text content of
the icon element. As a result, the name of the icon is included in the
parent element's `textContent`.

Previously, the `TocService` used to strip off these anchor elements
when generating the content of ToC items, but not when generating the
content of their tooltips. Thus, tooltips for ToC items would
confusingly include a `link` suffix (`link` is the textual name of the
icon used in heading anchor links).

This commit fixes this by deriving the tooltip content from the
transformed text content (which already has anchor links stripped off),
instead of from the original heading content.
The docs template for cli commands ([cli-command.template.html][1])
includes an `h2` element with GitHub links for [long description][2].
Since the content of `h2` elements is replicated in the auto-generated
Table of Contents, the GitHub links were replicated as well (which is
undesirable).

This commit fixes it by explicitly excluding `.github-links` elements,
when extracting the content for the ToC (in
[TocService#extractHeadingSafeHtml()][3]). This is similar to what we do
for the auto-generated `.header-link` anchors.

[1]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html
[2]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html#L18
[3]: https://github.com/angular/angular/blob/1537791f0/aio/src/app/shared/toc.service.ts#L56
@mary-poppins
Copy link

You can preview 0dd6d5a at https://pr32418-0dd6d5a.ngbuilds.io/.

@petebacondarwin petebacondarwin removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 5, 2019
@matsko matsko closed this in 007282d Sep 5, 2019
matsko pushed a commit that referenced this pull request Sep 5, 2019
)

The docs template for cli commands ([cli-command.template.html][1])
includes an `h2` element with GitHub links for [long description][2].
Since the content of `h2` elements is replicated in the auto-generated
Table of Contents, the GitHub links were replicated as well (which is
undesirable).

This commit fixes it by explicitly excluding `.github-links` elements,
when extracting the content for the ToC (in
[TocService#extractHeadingSafeHtml()][3]). This is similar to what we do
for the auto-generated `.header-link` anchors.

[1]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html
[2]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html#L18
[3]: https://github.com/angular/angular/blob/1537791f0/aio/src/app/shared/toc.service.ts#L56

PR Close #32418
matsko pushed a commit that referenced this pull request Sep 5, 2019
…32418)

The Table of Contents (ToC) is auto-generated based on the content of
heading elements on the page. At the same time, anchor links are
auto-generated and added to each heading element. Note that the Material
Icons used for the anchor icon make use of ligatures, which means that
the icons are specified by using their textual name as text content of
the icon element. As a result, the name of the icon is included in the
parent element's `textContent`.

Previously, the `TocService` used to strip off these anchor elements
when generating the content of ToC items, but not when generating the
content of their tooltips. Thus, tooltips for ToC items would
confusingly include a `link` suffix (`link` is the textual name of the
icon used in heading anchor links).

This commit fixes this by deriving the tooltip content from the
transformed text content (which already has anchor links stripped off),
instead of from the original heading content.

PR Close #32418
matsko pushed a commit that referenced this pull request Sep 5, 2019
)

The docs template for cli commands ([cli-command.template.html][1])
includes an `h2` element with GitHub links for [long description][2].
Since the content of `h2` elements is replicated in the auto-generated
Table of Contents, the GitHub links were replicated as well (which is
undesirable).

This commit fixes it by explicitly excluding `.github-links` elements,
when extracting the content for the ToC (in
[TocService#extractHeadingSafeHtml()][3]). This is similar to what we do
for the auto-generated `.header-link` anchors.

[1]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html
[2]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html#L18
[3]: https://github.com/angular/angular/blob/1537791f0/aio/src/app/shared/toc.service.ts#L56

PR Close #32418
@gkalpak gkalpak deleted the fix-aio-gh-links-in-toc branch September 5, 2019 17:42
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
…ngular#32418)

The Table of Contents (ToC) is auto-generated based on the content of
heading elements on the page. At the same time, anchor links are
auto-generated and added to each heading element. Note that the Material
Icons used for the anchor icon make use of ligatures, which means that
the icons are specified by using their textual name as text content of
the icon element. As a result, the name of the icon is included in the
parent element's `textContent`.

Previously, the `TocService` used to strip off these anchor elements
when generating the content of ToC items, but not when generating the
content of their tooltips. Thus, tooltips for ToC items would
confusingly include a `link` suffix (`link` is the textual name of the
icon used in heading anchor links).

This commit fixes this by deriving the tooltip content from the
transformed text content (which already has anchor links stripped off),
instead of from the original heading content.

PR Close angular#32418
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
…ular#32418)

The docs template for cli commands ([cli-command.template.html][1])
includes an `h2` element with GitHub links for [long description][2].
Since the content of `h2` elements is replicated in the auto-generated
Table of Contents, the GitHub links were replicated as well (which is
undesirable).

This commit fixes it by explicitly excluding `.github-links` elements,
when extracting the content for the ToC (in
[TocService#extractHeadingSafeHtml()][3]). This is similar to what we do
for the auto-generated `.header-link` anchors.

[1]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html
[2]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html#L18
[3]: https://github.com/angular/angular/blob/1537791f0/aio/src/app/shared/toc.service.ts#L56

PR Close angular#32418
arnehoek pushed a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
…ngular#32418)

The Table of Contents (ToC) is auto-generated based on the content of
heading elements on the page. At the same time, anchor links are
auto-generated and added to each heading element. Note that the Material
Icons used for the anchor icon make use of ligatures, which means that
the icons are specified by using their textual name as text content of
the icon element. As a result, the name of the icon is included in the
parent element's `textContent`.

Previously, the `TocService` used to strip off these anchor elements
when generating the content of ToC items, but not when generating the
content of their tooltips. Thus, tooltips for ToC items would
confusingly include a `link` suffix (`link` is the textual name of the
icon used in heading anchor links).

This commit fixes this by deriving the tooltip content from the
transformed text content (which already has anchor links stripped off),
instead of from the original heading content.

PR Close angular#32418
arnehoek pushed a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
…ular#32418)

The docs template for cli commands ([cli-command.template.html][1])
includes an `h2` element with GitHub links for [long description][2].
Since the content of `h2` elements is replicated in the auto-generated
Table of Contents, the GitHub links were replicated as well (which is
undesirable).

This commit fixes it by explicitly excluding `.github-links` elements,
when extracting the content for the ToC (in
[TocService#extractHeadingSafeHtml()][3]). This is similar to what we do
for the auto-generated `.header-link` anchors.

[1]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html
[2]: https://github.com/angular/angular/blob/1537791f0/aio/tools/transforms/templates/cli/cli-command.template.html#L18
[3]: https://github.com/angular/angular/blob/1537791f0/aio/src/app/shared/toc.service.ts#L56

PR Close angular#32418
@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 Oct 6, 2019
@gkalpak gkalpak added this to IN PROGRESS in docs-infra Oct 11, 2019
@gkalpak gkalpak moved this from IN PROGRESS to MERGE in docs-infra Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: patch This PR is targeted for the next patch release type: bug/fix
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants