Revert hover link approach to fix markup of headings containing links

[DavidOliver]

Fixes 28bb43e#r125061077

Also makes the link underline color the same as the text (usinig currentColor, which is black or white) for admonition headings.

[josevalim]

💚 💙 💜 💛 ❤️

[josevalim]

Hi @DavidOliver, I am sorry but I went back and revert this. Because with this change, we can no longer click on the title to get the anchor. So I prefer to have broken but functional markup, then correct markup that is non-functional. :)

[DavidOliver]

@josevalim I understand. My thinking was that obtaining the fragment identifier is easy enough by clicking on the link icon. If I'm able to add the behaviour you would like via JS, would you be open to then accepting so that malformed headings/links are not broken into separate anchors, and so that links in headings are correctly rendered?

[DavidOliver]

@josevalim I thought about your message again, and I realised that I'm not sure whether or not you realised the link to the section heading can still be obtained by clicking on the link icon to the left which appears on hover. No functionality has been lost with this change, and it does fix a significant problem with links in headings, so I think it's better with the fix than without. :)

[josevalim]

I am worried about mobile devices, given the anchor only appears on mouse over, and being less discoverable in general. But fixing it with JS would also be fine by me. :)

There are other places we don't show the anchor, such as cheatsheets and admonitions, so we would need answers for those to if we don't add the JS.

[DavidOliver]

@josevalim, got it. I'll give it some more thought and experiment.

Maybe we show the link icon semi-transparently until hover?

[josevalim]

I think it feels really cluttered on mobile. :(

[DavidOliver]

@josevalim, sorry for the delay. I have a new approach in progress that makes only markup and CSS changes and that preserves the fragment identifier/section link on hover of the main heading text. It makes basic use of CSS Grid to overlay the heading text on the section link (the overall hoverable area), and sets pointer-events to achieve the desired behaviour of the two types of links in play.

I'll submit a new PR when I've tidied and tested in more browsers/devices.

[josevalim]

Thats awesome @DavidOliver. No worries and thank you for the updates and your contributions!

[DavidOliver]

@josevalim, before I update the tests and submit a PR, could you check that you're happy with the appearance and markup, please?

The hover link's chain icon appears only when hovering a part of the heading that is not itself "a link".

I've restored the hover link chain icon for cheatsheet H3s.

Example HTML:

[josevalim]

Hi @DavidOliver, this looks good to me. As long as it looks and behaves the same as today, we should be more than fine. Also, make sure to see the generated epub and check if it looks good too. :)

[DavidOliver]

@josevalim, given the approach above, headings in the epub appear like this:

So the heading itself, generated via ExDoc.Formatter.HTML.Templates.link_heading/5 is no longer wrapped in an <a>, meaning there's no 'click to navigate to this heading itself' in the epub reader. (Author-defined links within the heading are, of course, displayed as anchors, now with valid markup.)

I think this may not be a problem as the epub nav.xhtml Table of contents page does not contain sub-page links, and I'm not sure if epub readers generally support opening up a book and navigating to a fragment/element via an id anyway? (There is a more advanced fragment identifier proposal for epub which is not dependent on ids, e.g., book.epub#epubcfi(/6/4[chap01ref]!/4[body01]/10[para05]/3:10).)

Module pages' heading markup, generated via an epub-specific template, remains untouched:

So, for example, the 'Callbacks' and 'Functions' headings here continue to link to the relevant sections below.

Is this all okay to go ahead with?

[josevalim]

Yes, it is fine. None of those links are useful because they mostly exist for getting the anchor. We could also remove the links from Summary/Function/etc in the ebook. Would you like to go ahead with this change too? Otherwise I can open a separate issue. :) Thank you!

[DavidOliver]

@josevalim, great - thanks. Yes, I'll submit a separate PR for removing the self-pointing links in epub module page headings.