-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Support a plugin to allow clickable sections that lead to an index page #2060
Comments
@waylan – TL;DR: navigation sections as pages, i.e. index pages – you have invested quite some time into this topic. If you, maybe, could comment whether the proposed plugin solution, which as I understand, is mainly a deduplication of index pages, could be integrated natively into MkDocs, that would be amazing. I'm not convinced that this functionality should be a plugin. Thanks again for all the detailed work on this. You might have seen that Material for MkDocs is available in two editions - the regular one and Insiders, the latter of which is only accessible to sponsors of this project. Insiders contains a lot of new functionality that evolves around navigation, namely: Furthermore, Material for MkDocs already supports the following feature flags to change navigation: All of these flags impact the way, navigation is rendered. In order to integrate the changes you proposed, we need to make sure that they don't impact the functionality of any flag on mobile (drawer) and desktop (sidebar) navigation. Also note that those flags can be used in any combination - they're not mutually exclusive. Some of them are currently only available to sponsors (Insiders), but we must make sure that Insiders is also compatible. Unfortunately, my schedule for Insiders is pretty packed and I don't expect to have time to move your feature suggestion forward within the next three months. Luckily, I'm pretty confident that this funding goal might be hit within those three months, so all navigation feature flags will be merged back into Would this work for you? |
Hi. Thanks for the consideration. I do not plan to interact with the sponsoring, so waiting it is, then. If the plugin in the meantime manages to cause invalid support requests to be sent to you, (1) I hope it's not too bad and (2) of course feel free to redirect them to me. |
This is what mkdocs/mkdocs#1042 was, and that was ultimately rejected for the reasons stated there. Since then, the only effort I have put into the matter is writing this proposal for how I imagine a theme could solve the issue with no changes to MkDocs. What I have not seen is any feedback on that proposal. Have theme authors attempted it? If so, what is unsatisfactory about it? |
@waylan I think what the OP is suggesting is that he wants to reduce template "magic" that needs to be applied by themes, i.e. the deduplication of the index entry which you suggested in your proposal. However, I don't see any big issue with it. Thus, from what I understand, the plugin reduces the need for template authors to perform the necessary deduplication, which in itself shouldn't be that big a deal. |
Yes, and that is exactly what I created and then rejected in mkdocs/mkdocs#1042. The issue is that there is no reasonable way to make that structure work with the builtin themes (at least not without a major redesign). And therefore, I'm not inclined to include support in the underlying data structure. Whether such a feature is accomplished through a plugin or template "magic" is the choice of whoever implements it. However, if someone wanted to tackle a major redesign of the builtin theme which did support the feature, then I might reconsider. |
@waylan thanks! I don't expect this to happen any time soon, so we'll investigate plugin or template support. |
The funding goal has been reached and the respective features and fixes have just been merged from Insiders, thus we could now tackle this proposal/enhancement. |
This wasn't badly affected by the code change. The current diff is this: diff --git a/material/partials/nav-item.html b/material/partials/nav-item.html
index 8a90ad49c..e90e1fc8a 100644
--- a/material/partials/nav-item.html
+++ b/material/partials/nav-item.html
@@ -20,13 +20,23 @@
<input class="md-nav__toggle md-toggle" data-md-toggle="{{ path }}" type="checkbox" id="{{ path }}" {{ checked }}>
{% endif %}
<label class="md-nav__link" for="{{ path }}">
- {{ nav_item.title }}
<span class="md-nav__icon md-icon"></span>
+ {% if nav_item.url %}
+ <a href="{{ nav_item.url | url }}" class="md-nav__link{% if nav_item == page %} md-nav__link--active{% endif %}"
+ style="margin: initial; padding: initial; pointer-events: initial">
+ {% endif %}
+ {{ nav_item.title }}
+ {% if nav_item.url %}</a>{% endif %}
</label>
<nav class="md-nav" aria-label="{{ nav_item.title }}" data-md-level="{{ level }}">
<label class="md-nav__title" for="{{ path }}">
<span class="md-nav__icon md-icon"></span>
+ {% if nav_item.url %}
+ <a href="{{ nav_item.url | url }}" class="md-nav__link{% if nav_item == page %} md-nav__link--active{% endif %}"
+ style="margin: initial; padding: initial; pointer-events: initial">
+ {% endif %}
{{ nav_item.title }}
+ {% if nav_item.url %}</a>{% endif %}
</label>
<ul class="md-nav__list" data-md-scrollfix>
{% set base = path %}
@@ -47,8 +57,13 @@
{% endif %}
{% if toc | first is defined %}
<label class="md-nav__link md-nav__link--active" for="__toc">
- {{ nav_item.title }}
<span class="md-nav__icon md-icon"></span>
+ {% if nav_item.url %}
+ <a href="{{ nav_item.url | url }}" class="md-nav__link{% if nav_item == page %} md-nav__link--active{% endif %}"
+ style="margin: initial; padding: initial; pointer-events: initial">
+ {% endif %}
+ {{ nav_item.title }}
+ {% if nav_item.url %}</a>{% endif %}
</label>
{% endif %}
<a href="{{ nav_item.url | url }}" class="md-nav__link md-nav__link--active">
diff --git a/material/partials/tabs-item.html b/material/partials/tabs-item.html
index c05954986..31d3bd864 100644
--- a/material/partials/tabs-item.html
+++ b/material/partials/tabs-item.html
@@ -7,10 +7,10 @@
{% set class = class ~ " md-tabs__link--active" %}
{% endif %}
{% endif %}
-{% if nav_item.children %}
+{% if nav_item.children and not nav_item.url %}
{% set title = title | d(nav_item.title) %}
{% set nav_item = nav_item.children | first %}
- {% if nav_item.children %}
+ {% if nav_item.children and not nav_item.url %}
{% include "partials/tabs-item.html" %}
{% else %}
<li class="md-tabs__item"> Note that the diff doesn't try to be a good refactor but is instead made to be easily auto-applied on the fly. I apply it here: https://github.com/oprypin/mkdocs-section-index/blob/e04d0c4ed5298cde8d07a33c94173f0d5a21ee18/mkdocs_section_index/rewrites.py#L44-L65 An actual PR would look different. Though I'm not sure if it would be easy for me to make a good refactor. Demo site |
Thanks for the diff. I just saw that you're putting links ( |
I believe that this can be fully implemented without the use of a plugin, and it actually should be, as it's a template matter after all. There's a property which is called Take the following nav:
- Home: index.md
- Le Foo:
- Le Bar: foo-what-ever/bar.md
- Index: foo-what-ever/index.md
- External: http://google.com Result: For Material for MkDocs, the implementation may be tricky, as there are a lot of cases to consider due to the different navigation options there are, but in general, it should be trivial to support, at least from my current understanding. It will also work if you omit titles, which might be desirable for Index pages, as they should take the titles from their section: nav:
- index.md
- Le Foo:
- foo-what-ever/index.md
- Le Bar: foo-what-ever/bar.md
- External: http://google.com |
Never argued that it can't
Well, not really. MkDocs simply has no concept of a page being associated with a section. And a job of a template is to represent the concepts that MkDocs does have. If a template decides to do something that is not represented in the nav, that would technically just be wrong.
I say "it's certainly not as easy as just checking against Do you think it's obvious what the index should be in this scenario? nav:
- index.md
- Le Foo:
- Maybe: other/index.md
- le-foo/index.md
- Le Bar: le-foo/bar.md My opinion still is that this feature should be explicitly opted into (which yes, it's possible to have configuration of a template), but I want to make it portable across themes; maybe one would want to switch out a theme without losing this. And the other part of my point still is that all the templates would have to implement this config themselves and the messy logic of finding and excluding an index page, in template code, each possibly in a different way. Having said that, maybe it's not such a bad thing to give each theme the ability to decide what an index page means for it, but it's still probably more confusing than useful. |
Thanks for providing a counterexample. This is indeed a valid concern. However, it doesn't happen when you don't specify the
I agree completely, which is why I'd hide it behind a feature flag as I've done with all new features.
I believe that making it portable is a Sisyphean task, as the representation of navigation is a theme-problem, not a plugin problem. Sure, your plugin helps to normalize the data structure which gets passed to the template which is great, but I really don't want to create another dependency for such a fundamental change. If you decide to change something on your plugin, all themes would need to adapt, and as the implementation has to happen in the theme (monkey-patching aside), it creates more moving parts.
This is exactly what I mean by theme-dependent. It heavily depends on the representation. Nonetheless: thanks for your work on this topic and for pushing this so far. It's a matter I've long decided is not implementable, but it turns out it is. That's pretty great, as it may benefit many users. I've put index pages on the roadmap. |
Representation of a navigation state that literally can't be specified is no longer a theme-problem.
Why would I be so horrible :o
I wouldn't really call it that. The theme would just support the protocol, which is a guarantee that can be as strong as mkdocs protocol itself. The only thing that requires more setup is testing for regressions, but I think that would be felt mostly from my side, and I have quite robust tests for it now. Complaints would go to my plugin hopefully, anyway.
Right. It doesn't. Just makes everything very explicit, and if it turns out that users keep presenting different use cases for how this index selection needs to be configurable, that will need to be implemented only once, not in every theme (and not inside Jinja templates). Really the main issue that I have here is my plugin would still be needed for other themes, meanwhile your theme would do such a complicated and incompatible manipulation that I would have no choice but to somehow detect the theme and skip the plugin's operation for it entirely. |
Then I'd not regard it as a plugin problem, but an engine (= MkDocs) problem for something so fundamental that is navigation. However, I don't understand the argument you're making. The counterexample you gave provides an ambiguous index for a section, thus you say that you cannot use
Essentially, only lines 2-4 are new here. Checking if the child is not an index could easily be done with scope (in Jinja called context), thus persisting the index child in a scoped variable, so that when rendering the children this variable can be checked. Material for MkDocs does something similar for rendering its tabs navigation. It will recurse into the first child until a non-section is found, checking and persisting the mkdocs-material/src/partials/tabs-item.html Lines 23 to 56 in afff64d
As the logic I'm evaluating to integrate for merging index pages will be opt-in (= feature flag), you can still offer your plugin and monkey-patch the sources. It will land in Insiders first, so non-Insiders users can still benefit from your plugin. In general, I don't want to maintain more upstream dependencies, and yes, the navigation partial is rather complex as there are a lot of options with this theme. Thus, the one maintaining it would be me + address the problems that may be introduced through future incompatibilities between MkDocs and your plugin. I guess we're reliving the whole discussion waylan already had several times 😅 |
OK well now the conflict of interest is clear. That gives you the ability to advertise a paid feature, possibly without acknowledging that a longer-standing and free alternative exists -- which even makes sense because you don't want to be responsible for responding to issues with it. (which, to be clear, I never asked for the latter).
Yeah thanks for the extra maintenance work |
All new features land in Insiders, that's the model I'm following to make the development and maintenance on this project sustainable. Before, I've invested months and months and months of work into this project without any financial returns. What started out as a template I wanted to use for my Open Source projects became so large, it was flooded with issues and feature requests, many of which I implemented for free. The stars, unfortunately, won't pay my rent so I needed to find a model that's sustainable or I wouldn't be able to maintain this project in the long term. All the new features that came with 6.2 are only there because of Insiders, and they are available to the public, so no, there's no conflict of interest. It's essentially the only model that allows me to give my continued attention to this project. Furthermore, Insiders includes integrations to third-party features / plugins / utilities, like for example mike for versioning, so I definitely have an interest to link to third-party features when I feel that it makes sense. You can also use mike's monkey-patched version selector integration, but it's obviously not as native as the one provided with Insiders. I'm sorry if this model doesn't align with your goals, but after evaluating the possibility of including your plugin I see no upside in doing so, as I outlined in my comments. You're always free to fork this project, it is a free offer after all.
Regardless of where this feature would land, I didn't promise at any point to integrate your patches. Please try to imagine how hard it is to maintain a template with so many users and balance so many conflicting interests in something that is still maintainable. I didn't expect a template to become so popular and so useful, and the more options, dependencies and indirections we have, the more complicated it gets, so pardon my decision, but I think it is the right one. I may, however, re-evaluate in the future, as I've done with |
Insiders 1.17.0 adds native support for section index pages. Note that it was trickier to implement than expected. The new implementation works out of the box with navigation tabs + sections and produces valid HTML, i.e. combines the link with the expandable chevron. EDIT: @oprypin I added a footnote with a link to your plugin, as an alternative to Insiders. |
That's exactly what I've been missing out (after migrating from Antora!) Big thank you! |
Awesome, any feedback appreciated 😎 |
- Well it's something :/ You could give it a liiittle more of a spotlight than that small digit, but I guess you don't have to. And you also say
That's just a bit misleading. I literally have tests for that and I also test manually on every change to the file. |
Thanks though |
@squidfunk Think the URL is incorrect --> localhost |
@squidfunk I can't get it to work. Does it really only work when using subfolders and nav:
- Section:
- section-overview.md
- Page 1: page-1.md
...
- Page n: page-n.md |
@wilhelmer The description of the new feature says it has to be index.md. My plugin doesn't have that limitation. (Unless Insiders breaks it, but no way for me to know) |
Sorry, I should've read the thread before posting, I see there's been a lot of buzz around this feature, obviously (and a little surprising to me) it's not easy to implement this in a simple fashion. @oprypin Your plugin doesn't have the |
No |
Well, maybe you should demonstrate it without subfolders then, because that's even more awesome ;-) |
@jaceklaskowski it was the wrong link in the issue here, yeah, haven't seen the localhost part when I copied it. This is now fixed in my comment. |
@oprypin I understand that you're not satisfied that I took a different road than you suggested. However, I have no bandwidth to officially support your plugin, and I believe it is not the most ideal approach to tackle this problem, as I've outlined in my previous comments. Some of the issues I've outlined, Yes, the file has to be named As your plugin is not officially supported, I believe the footnote is sufficient to outline that there are alternatives. There are many plugins that work with Material for MkDocs out-of-the-box, but are not officially supported and they are not listed in the documentation. I will adjust the footnote, so it says, that there might be incompatibilities with certain features, because I cannot guarantee that there are not. |
You have no bandwidth to write conditionals that account for both
You're complaining about how I implement a hack needed only because you won't cooperate. It's infeasible for me to maintain a huge refactor when the source keeps changing without any notice, especially in ways hostile towards this. Of course, if you didn't start off with a guarantee to throw away my work, I would've addressed this. As is, I preferred a smaller patch over a refactor.
Tabs have been supported for 2 months now - soon after the project started. Navigation sections also work -- I think that was supported even before tabs. Why do you keep claiming things don't work? Moreover, there's nothing in the core of the plugin that would ever prevent supporting any of this, just need the HTML to cooperate a little.
Same with my plugin.
Well I have tested and it works with my plugin.
Yes, it is. Also great that it's possible to add configuration options non-intrusively. What's not great is that it's a huge amount of work to maintain patches of your theme. And when I say it, it's actually true, unlike your claims that my plugin adds maintenance work. I am sure it would reduce the work even for you. You said yourself:
-- well, the alternative would've been almost no implementation work at all. As well as simpler long-term maintenance of the logic.
Just like a plugin is opt-in? Both are 1-line additions to the config. |
There are a lot of allegations in your last comment, i.e. that my work is hostile towards yours. This project, including the Insiders branch, is an offer. You don't have to take that offer, and you don't have to agree with all of my decisions. You're always free to walk away and use another technology that solves the same problem. Other than that, you can also fork this project and maintain your own fork that includes all of the changes you need. I just don't believe your solution is the best possible fit for this project. Please accept that. I'm locking the conversation because the debate is pointless. |
I have developed a plugin for MkDocs, "section-index" that allows sections in the left-side nav to be not just expandable, but also have a link to a page directly associated with them.
The plugin already fully works as-is (you can use it!), but the implementation currently needs to "hack into" the templates of themes (such as mkdocs-material and others). So I am reaching out to ensure good long-term support of this idea.
mkdocs-material theme could integrate optional "code paths" directly into its templates in a way that has 0 effect when the plugin isn't loaded, and if it is loaded, the template directly works with the "protocol" provided by it, without hacks.
Demos
A real website using this
Screencast with comparison
The above demo features this^ mkdocs.yml config. Obviously by default in MkDocs this means something else, but with this plugin, the borgs/index.md page is merged as the index of the "Borgs" section.
Details
Other than the part that rewrites theme templates, the core of this plugin is the code that modifies the nav object. When it encounters a Section and a Page in a configuration like above, it collapses the two into one object, which is a hybrid of properties of both:
title
:str
url
:str
(normally exclusive to Page)children
:list
(normally exclusive to Section)is_page
=True
is_section
=True
The condition for this collapsing is up for discussion, but currently it's very "permissive". Just any page that's specified only as a link (no title), if it's the first item of a section, becomes merged as the index page of the section. The idea is for people to always specify names for all pages in the nav except for the section-title ones.
Other approaches are possible, but it's certainly not as easy as just checking against
index.md
.With this, all that a theme needs to do is consider the case where both
url
andchildren
are present (without this plugin that can never happen), and render a "section-page" as part of the navigation menu.An individual theme could actually implement this feature on its own (no plugin), by checking for such conditions directly within Jinja templates, but I personally find that less clean. See my further arguments on this.
The text was updated successfully, but these errors were encountered: