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

Wrap heading content support? #5

Closed
XhmikosR opened this issue Nov 20, 2018 · 4 comments · Fixed by #7
Closed

Wrap heading content support? #5

XhmikosR opened this issue Nov 20, 2018 · 4 comments · Fixed by #7
Labels
enhancement New feature or request

Comments

@XhmikosR
Copy link

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
Copy link
Contributor

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
Copy link
Owner

allejo commented Nov 20, 2018

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?

@allejo allejo added the enhancement New feature or request label Nov 20, 2018
@MartijnCuppens
Copy link
Contributor

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 twbs/bootstrap#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
Copy link
Owner

allejo commented Nov 21, 2018

(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 😄

@allejo allejo closed this as completed in #7 Nov 25, 2018
@allejo allejo changed the title Wrap anchor tags support? Wrap heading content support? Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants