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

Allow slugify function for Tabbed to generate readable anchors #1807

Closed
squidfunk opened this issue Sep 14, 2022 · 16 comments · Fixed by #1811
Closed

Allow slugify function for Tabbed to generate readable anchors #1807

squidfunk opened this issue Sep 14, 2022 · 16 comments · Fixed by #1811
Labels
T: feature Feature.

Comments

@squidfunk
Copy link
Sponsor Contributor

squidfunk commented Sep 14, 2022

Description

Currently, ids generated by Tabbed are of the form __tabbed_{tabset}_{tab}, e.g. __tabbed_1_2. Material for MkDocs allows to add anchor links to tabs, which means you can now explicitly link to tabs. However, this generates rather ugly and non-permalink URLs: https://squidfunk.github.io/mkdocs-material/getting-started/#__tabbed_1_2

Benefits

Users asked whether the ids can be customized, and immediately the Attribute Lists extension popped up in my head. However, we probably don't need support for Attribute Lists, as the usage of a user-definable slug function would probably be enough. The slug function could be passed the contents of the tab label, and generate a readable anchor.

Solution Idea

For example, with this Markdown:

=== "This is a label"

    Content

The following id could be generated, which can be used as an anchor:

...
<input id="this-is-a-label" name="__tabbed_1" type="radio">
...

The Table of Contents extension also implements this, which is related, as it also generates anchor links. Configuration could possibly look like this:

markdown_extensions:
  pymdownx.tabbed:
    slugify: !!python/name:pymdownx.slugs.uslugify

And as a shortcut defaulting to the standard Markdown slugify function:

markdown_extensions:
  pymdownx.tabbed:
    slugify: true
@squidfunk squidfunk added the T: feature Feature. label Sep 14, 2022
@gir-bot gir-bot added the S: triage Issue needs triage. label Sep 14, 2022
@facelessuser
Copy link
Owner

I'll take a look and see. Sluggifying the IDs isn't the problem, but making sure they don't conflict with header IDs and such may or may not be so easy. I'll have to double-check how the uniqueness of header IDs is handled and if it takes into account other IDs. There may or may not be other complications, so I'll have to do some exploratory work first.

@gir-bot remove S: triage

@gir-bot gir-bot removed the S: triage Issue needs triage. label Sep 14, 2022
@facelessuser
Copy link
Owner

If we were to do this, I feel we'd need to run our own tree processor, either before or after TOC. We'd have to take note of all existing IDs in the tree, and then generate our unique IDs, ensuring we do not duplicate any that we found or create in the process.

TOC slugifies header IDs, and does this pattern itself. We too would need to mimic this approach. TOC will not do this for us, and we can't really inject our logic into TOC, so it would need to be a separate pass just for tabs.

@facelessuser
Copy link
Owner

Yeah, we would have to slugify and then update the id of the input and the for of the label. This is a bit more involved for alternate tab style vs legacy, but obviously, completely doable.

I don't think this affects any of our JS examples as we do not hard code looking for __tabbed.

@facelessuser
Copy link
Owner

I was able to prototype something up:

import markdown
from pymdownx.slugs import uslugify

PROTO = """
### Here is some text

=== "Here is some text"
    content

=== "Here is some text"
    content
"""


print(markdown.markdown(
    PROTO,
    extensions=['pymdownx.tabbed', 'markdown.extensions.toc'],
    extension_configs={
        'pymdownx.tabbed': {'alternate_style': True, 'slugify': uslugify, 'separator': '-'},
        'markdown.extensions.toc': {'slugify': uslugify}
    }))
<h3 id="here-is-some-text">Here is some text</h3>
<div class="tabbed-set tabbed-alternate" data-tabs="1:2"><input checked="checked" id="here-is-some-text_1" name="__tabbed_1" type="radio" /><input id="here-is-some-text_2" name="__tabbed_1" type="radio" /><div class="tabbed-labels"><label for="here-is-some-text_1">Here is some text</label><label for="here-is-some-text_2">Here is some text</label></div>
<div class="tabbed-content">
<div class="tabbed-block">
<p>content</p>
</div>
<div class="tabbed-block">
<p>content</p>
</div>
</div>
</div>

@facelessuser
Copy link
Owner

I'll have to decide if we add this to Tabbed extension or the new generic block implementation of tabs...or both I guess. Anyways, I at least have code that works and will figure things out moving forward.

@squidfunk
Copy link
Sponsor Contributor Author

Nice! Looks great! I'd vouch for adding it experimentally to Tabbed since I imagine that a lot of users would immediately benefit and if that works well, add it to the generic block extension as well.

@facelessuser
Copy link
Owner

The one thing to note is that we were never going to pull this off with just attr_list I completely forgot about having to update for as well.

I do think slugifying the IDs is probably more helpful than a user having to manually define IDs as well.

Anyways, I think we can target the next release with this as this shouldn't be much work to cleanup and finish.

@squidfunk
Copy link
Sponsor Contributor Author

Jup, I also think slugification is superior to manually defining IDs, albeit there are edge cases where slugs are not stable, e.g. when a headline with the same name as a tab label is inserted before the tab container. However, I think that's a risk worth taking. Looking forward to the release!

@squidfunk
Copy link
Sponsor Contributor Author

BTW, if you can push this to a branch, I'm happy to test it.

@facelessuser
Copy link
Owner

I'll try to get something out today or tomorrow. I'll ping you when it is available. It's been a crazy busy couple of weeks for me 😅.

@squidfunk
Copy link
Sponsor Contributor Author

No worries, I didn't want to rush you. Take your time and just ping me!

@facelessuser
Copy link
Owner

facelessuser commented Sep 15, 2022

You weren't, more just mentioning it in case I don't deliver on what I just said 🙃.

@facelessuser
Copy link
Owner

@squidfunk #1811

@facelessuser
Copy link
Owner

As far as I can tell, it is working well, simply add the following config:

  - pymdownx.tabbed:
      alternate_style: true
      slugify: !!python/object/apply:pymdownx.slugs.slugify {kwds: {case: lower}}

Don't forget that things like uslugify are deprecated and that slugify is the preferred way to get and configure your slugs: https://facelessuser.github.io/pymdown-extensions/extras/slugs/#slugs.

I will be writing up documentation, and I think I will merge the solution closing this issue.

@squidfunk
Copy link
Sponsor Contributor Author

squidfunk commented Sep 18, 2022

Slugification indeed works, pretty cool! There are some problems that need to be fixed in Material for MkDocs before we can recommend this setting. Nothing serious, just some CSS needs to be adjusted, as well as the JavaScript that adds links.

Edit: I think I found all issues and fixed them in squidfunk/mkdocs-material-insiders@47477622b.

Don't forget that things like uslugify are deprecated and that slugify is the preferred way to get and configure your slugs: https://facelessuser.github.io/pymdown-extensions/extras/slugs/#slugs.

Ah, I didn't know that. Could you maybe PR the necessary changes to our docs? We use uslugify in several places, i.e. here (maybe just PR one section, I'll fix up the rest then):

@squidfunk
Copy link
Sponsor Contributor Author

Thanks again! I've enabled it on our site and updated the documentation in squidfunk/mkdocs-material@270f37230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: feature Feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants