allejo / jekyll-anchor-headings

A GitHub Pages compatible way of adding anchors to your headings without a plug-in or JavaScript :octocat:
https://pure-liquid.allejo.org/
MIT License
186 stars 31 forks source link

Wrap heading content support? #5

Closed XhmikosR closed 5 years ago

XhmikosR commented 5 years ago

Hey, @allejo .

I'm gonna switch to your package for Bootstrap. While at it, I noticed we could use an optional element wrapping support for the a tags.

Do you think this would be possible without too many changes?

Thanks in advance!

/CC @MartijnCuppens

MartijnCuppens commented 5 years ago

Maybe sketch a bit why we need this. We have a fixed header on our documentation pages (http://getbootstrap.com/docs/4.1/getting-started/introduction/#quick-start). With the inner wrapping div we set a title offset so that the title doesn't go below the navigation.

allejo commented 5 years ago

So what's currently being generated is this:

<h1>
    My Heading
    <a href="#heading">#</a>
</h1>

Which of these options are you talking about being generated?

<h1>
    My Heading
    <div>
        <a href="#heading">#</a>
    </div>
</h1>
<h1>
    <div>
        My Heading
        <a href="#heading">#</a>
    </div>
</h1>

Should be simple enough to add, but couldn't this be solved with a ::before to the headings?

MartijnCuppens commented 5 years ago

Last option.

We use a ::before at the moment, but because of that, we overlap some content before the title. To make sure the content before the title is still interact-able (eg clicking links), we set pointer-events: none to the titles. Then we add pointer-events: auto to the wrapping <div> to make links in the title clickable.

Ow, I also think it's better to use a <span> instead of a <div>, a <div> is not valid (something we've also just changed in Bootstrap https://github.com/twbs/bootstrap/pull/27695).

And it would be nice if we can also add a class to the wrapper. 😄

(Btw, we're just asking this because this would be handy to use in the Bootstrap docs, totally fine if you decide not to implement this)

allejo commented 5 years ago

(Btw, we're just asking this because this would be handy to use in the Bootstrap docs, totally fine if you decide not to implement this)

If it's useful to Bootstrap docs, I'm sure someone else will have similar needs so it's worth investing time into implementing this feature 😄