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

Content tabs from different modules in the same markdown cross link to the first one always #193

Closed
zuru opened this issue Dec 10, 2020 · 13 comments

Comments

@zuru
Copy link

@zuru zuru commented Dec 10, 2020

Not sure if this is related to mkdocs, mkdocstrings, or pytkdocs.
When using content tabs with the Tabbed extension:

markdown_extensions:
  - pymdownx.tabbed

within different module docstrings, which are rendered on the same page, their IDs are regenerated and the following content tabs link to the first content tabs when clicked.

To reproduce, add a content tab block in the docstring of two modules:

=== "Tab1"
   some content

=== "Tab2"
   some content2

and then add the modules in the same markdown:

::: module1

::: module2
@pawamoy
Copy link
Member

@pawamoy pawamoy commented Dec 21, 2020

Hello @zuru, thank you for the report!

I was able to confirm this is a bug, unfortunately I'm going to mark this as "won't fix" for now because we (@oprypin and I) don't see an obvious solution to this.

To summarize, the bug happens because we render each Markdown docstring separately, so we can't keep track of previous tabs with same title and therefore they end up with duplicated IDs. The last tabs then always redirect to the first ones with same IDs.

@oprypin has been fixing similar bugs and implementing similar features using custom Markdown extensions, but we fear (correct me if I'm wrong @oprypin!) this issue would need weird hacks that would complicate the code too much. I don't really want to alter the whole Markdown rendering process, who knows what other issues stems from the fact we render each docstring separately...

So for now, what I did was add a note in the troubleshooting section of the docs (still not deployed) asking users to avoid using identical titles for different tabs that will be rendered in the same markdown page. Emphasis on same Markdown page because this issue does not happen when rendering tabs with identical titles on different pages.

I understand this can be annoying, but we don't have a solution for this yet 😕
Don't hesitate to share more on this subject of course!

@zuru
Copy link
Author

@zuru zuru commented Dec 21, 2020

Hi @pawamoy, thanks for checking it out. As a quick workaround, we used this js script to fix it:

///tab incremental id// 
function setID(){
    var tabs = document.getElementsByClassName("tabbed-set");
    for (var i = 0; i < tabs.length; i++) {
        children = tabs[i].children;
        var counter = 0;
        var iscontent = 0;
        for(var j = 0; j < children.length;j++){
            if(typeof children[j].htmlFor === 'undefined'){
                if((iscontent + 1) % 2 == 0){
                    //check if it is content
                    if(iscontent == 1){
                        btn = children[j].childNodes[1].getElementsByTagName("button");
                    }
                }
                else{
                //if not change the id
                children[j].id = "__tabbed_" + String(i + 1) + "_" + String(counter + 1);
                children[j].name = "__tabbed_" + String(i + 1) //+ "_" + String(counter + 1);
                //make default tab open
                if(j == 0)
                    children[j].click();
                }
                iscontent++;
            }
            else{
                //link to the correct tab
                children[j].htmlFor = "__tabbed_" + String(i+1) + "_" + String(counter + 1);
                counter ++;
            }
        }
    }
}
setID();
@zuru zuru closed this Dec 21, 2020
@pawamoy
Copy link
Member

@pawamoy pawamoy commented Dec 21, 2020

Oh well, this is great! Thanks a lot for sharing it! If you don't mind I will add it in the troubleshooting docs 🙂

pawamoy added a commit that referenced this issue Dec 21, 2020
As shared by @zuru in #193. Thanks!
@oprypin
Copy link
Member

@oprypin oprypin commented Dec 21, 2020

@pawamoy We might have misunderstood the scope of this issue. It's even worse, using different tab titles doesn't help!

image

With highlighted IDs:
image

Clicking "A Tab2" leads you to "Tab2" anyway!

Also the content of "A Tab1" doesn't show up at all, etc.

https://gist.github.com/6d82074d3539851e6eaec6dc8db72dd2

@oprypin
Copy link
Member

@oprypin oprypin commented Dec 21, 2020

So there's indeed a bit of a recurring theme with IDs in sub-docs. There was the one with footnotes, now with tabs. We will keep running into instances of this. For example, one can't even create two headings with the same name in two different docstrings, and still be able to link to them (including in the nav).

I think a general solution would go through the sub-docs in the end and prepend the doc identifier's id to them. That would fix those once and for all.

@oprypin
Copy link
Member

@oprypin oprypin commented Dec 21, 2020

If we look at this particular issue in isolation (disregarding what I said above), the core of it lies here:
https://github.com/facelessuser/pymdown-extensions/blob/1bc710051743178a608284c2b2cf31e722177b9b/pymdownx/tabbed.py#L230

The counter needs to be globally consistent, unless we decide to rewrite all IDs.

#195 actually went in the direction that's the opposite to this, it intentionally resets these. I thought it would actually be a regression in terms of this issue, but no, the behavior seems to be still as bad as before.

@oprypin
Copy link
Member

@oprypin oprypin commented Dec 21, 2020

If we're looking for a perfect fix, Python-Markdown could do a lot for this if it explicitly exposed an interface for nested sub-docs, and then extensions could each support that as well.

@oprypin
Copy link
Member

@oprypin oprypin commented Dec 22, 2020

I sent out a complete fix - "once and for all", as I say. #199

@pawamoy
Copy link
Member

@pawamoy pawamoy commented Dec 22, 2020

I see, I was too fast to conclude the titles were used to generate the IDs, my bad!
Thank you very much @oprypin for investigating all this and implementing a fix! This is absolutely great 🙂

I'll revert the docs troubleshooting notes, and we'll include this in v0.14 once you stop making such great improvements 😄

pawamoy pushed a commit that referenced this issue Dec 22, 2020
Issue #186: #186
Issue #193: #193
PR #199: #199
@zuru
Copy link
Author

@zuru zuru commented Dec 22, 2020

Great :) !

Do you have an estimate on when v0.14 will be available?

@pawamoy
Copy link
Member

@pawamoy pawamoy commented Dec 22, 2020

Not really, I'm doing my best to find and fix potential regressions since 0.13.6, and I'll release once satisfied. If it's not before 2021, it will be in January ^^

@pawamoy
Copy link
Member

@pawamoy pawamoy commented Dec 31, 2020

0.14.0b1 is out! Try it out and tell us if it works better 🙂

@zuru
Copy link
Author

@zuru zuru commented Jan 28, 2021

Many thanks just verified that it works, great job :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants