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

Refactor readthedocs navigation template. #1155

Merged
merged 1 commit into from Mar 11, 2017
Merged

Refactor readthedocs navigation template. #1155

merged 1 commit into from Mar 11, 2017

Conversation

@waylan
Copy link
Member

@waylan waylan commented Mar 10, 2017

  • Fix a nested <li> bug.
  • More closely replicate nesting of the Sphinx theme nav (related to #588).
  • If sections headers ever get urls, this will support them (related to #73).

I pulled the RTD changes out of #1042 and applied them separately with a few tweaks. These changes need to happen regardless of whether #1042 get rejected (my current leaning) and this gets us closer to resolving #588. The remaining work to complete #588 should be CSS/JS related only.

I should note that this is not an exact clone of the upstream (Sphinx) theme (although it is closer). In the upstream theme, there is only provision for one level of section headings and one level of pages were we allow pages in the section heading level and multiple levels of pages under a section. If we were to take it that far, then we would introduce backward incompatible changes to existing projects. For complete feature parity, that may be necessary, in which case this needs more work. If, on the other hand, we are willing to forego some feature parity for backward compatibility with our existing users, then this should be ready to go.

* Fix a nested `<li>` bug.
* More closely replicate nesting of the Sphinx theme nav (related to mkdocs#588).
* If sections headers ever get urls, this will support them (related to mkdocs#73).
@waylan
Copy link
Member Author

@waylan waylan commented Mar 10, 2017

For the record, here are the additional changes necessary for complete feature parity (see this comment for details):

  • Remove support for nesting nav.html inside hav.html.
  • Move top level nav headers out of ul and into p tags.
  • Each set of pages for a top level header must then be in a level1 ul (move up one level).
  • Explicitly exclude "homepage" from nav.
  • Document restrictions to pages structure (for rtd theme only):
    • The homepage is the only "page" allowed at root level.
    • All other root level items must be section headings.
    • Section headings cannot contain section headings (only one level of nesting).

It's the things listed in that last item which will break backward comparability for existing users. If they have not followed those rules, then the nav will be broken if they upgraded to a version which required them. And as this is all in the template, I don't see a clear migration path or way to issue warnings (aside from hard-coding some check in the core for a specific theme, which I'd rather avoid).

Loading

@waylan
Copy link
Member Author

@waylan waylan commented Mar 10, 2017

Having given this some thought, my suggestion is to merge this PR as-is (it does fix invalid HTML after all). If we can't complete #588 with the templates in this state, then the way forward for #588 would be to fork the theme and move forward with the fork under a new name, being careful to document the restrictions that the theme imposes on page structure, etc. If users want the benefits, then they need to rearrange their page structure to comply and then manually switch to the new theme. If the user (of the current theme) does nothing, then their site continues to work as it does now (albeit with cleaner HTML).

Loading

@waylan waylan merged commit a877aae into mkdocs:master Mar 11, 2017
3 checks passed
Loading
@simonvanderveldt
Copy link

@simonvanderveldt simonvanderveldt commented Jun 28, 2017

@waylan Was this supposed to enable/fix multiple levels of navigation for the readthedocs theme? When using multiple nested directories the nested directories aren't indented at all.

Loading

@waylan
Copy link
Member Author

@waylan waylan commented Jun 28, 2017

@simonvanderveldt no, this simply cleaned of the HTML generated by the template. Previously, the generated HTML was improperly nested and generally a mess. There is still much work to do with the CSS/JS for the RTD theme, which is the subject of #588.

Loading

@simonvanderveldt
Copy link

@simonvanderveldt simonvanderveldt commented Jun 28, 2017

@waylan OK, thanks for the info!

Loading

@waylan waylan deleted the rtd branch Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants