fix(aio): only remove the header-link from toc titles #22504
Conversation
petebacondarwin
added
type: bug/fix
action: review
|
You can preview 7c69b2e at https://pr22504-7c69b2e.ngbuilds.io/. |
aio/src/app/shared/toc.service.ts
Outdated
| const a = this.document.createElement('a') as HTMLAnchorElement; | ||
| a.innerHTML = heading.innerHTML; | ||
| const anchorLink = a.querySelector('a'); | ||
| const div = this.document.createElement('div') as HTMLAnchorElement; |
There was a problem hiding this comment.
The cast to HTMLAnchorElement is wrong (and seems unnecessary anyway).
gkalpak
added
action: cleanup
|
Actually, I think this PR is wrong as it stands as we should really be removing all links from the HTML that is copied over. Otherwise you have links inside a link which doesn't make sense, right? |
petebacondarwin
force-pushed
the
aio-remove-heading-anchor
branch
from
7c69b2e
to
dd39032
Compare
| throw new Error('$lastInfo is not yet defined. You must call `spyOn` first.'); | ||
| } | ||
| return this.$$lastInfo; | ||
| } |
There was a problem hiding this comment.
All this stuff is just to get around strict null checks, which are not yet turned on it seems.
petebacondarwin
added
action: review
|
@gkalpak PTAL |
|
You can preview dd39032 at https://pr22504-dd39032.ngbuilds.io/. |
| a.removeChild(anchorLink); | ||
| const div: HTMLDivElement = this.document.createElement('div'); | ||
| div.innerHTML = heading.innerHTML; | ||
| const anchorLinks: NodeListOf<HTMLAnchorElement> = div.querySelectorAll('a'); |
There was a problem hiding this comment.
OOC: Are the HTMLDivElement (above) and NodeListOf<HTMLAnchorElement> types necessary? Latest TS can infer them. (Maybe time to upgrade 😁)
There was a problem hiding this comment.
'tis true but we haven't upgraded yet!
gkalpak
added
action: cleanup
|
Just realised that I need to change the commit message - since the solution is now different. I will rebase and squash when doing so. |
The previous approach just removed the first `a` tag that was found, but now that the header-link anchor is not at the start of the heading, it could fail. Closes angular#22493
petebacondarwin
force-pushed
the
aio-remove-heading-anchor
branch
from
dd39032
to
69fd953
Compare
petebacondarwin
added
action: merge
|
You can preview 69fd953 at https://pr22504-69fd953.ngbuilds.io/. |
|
I accidentally deleted the branch for this PR, which closed this PR. I reinstated the branch but I don't seem to be able to reopen the PR. I will open a new one. |
|
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. |
The previous approach just removed the first
atag thatwas found, but now that the header-link anchor is not at
the start of the heading, it could fail.
Closes #22493