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

Move layout/sidebar into frame.html.jinja2 #346

Closed
denised opened this issue Feb 9, 2022 · 9 comments
Closed

Move layout/sidebar into frame.html.jinja2 #346

denised opened this issue Feb 9, 2022 · 9 comments
Assignees

Comments

@denised
Copy link
Contributor

denised commented Feb 9, 2022

Problem Description

In my pdoc-powered site, I wished to combine elements of pdoc and non-pdoc content on the same pages. In order to do this, I had to set the 'module' variable, even though it didn't make sense in this context. It is possible to work around, but somewhat awkward.

Proposal

Go through the code and find every place that module is used, and ensure that the code would work with a null object instead. This might perhaps be limited to the nav part of the template (leaving the main part of the template breaking if module isn't an actual value, which kinda makes sense).

Alternatives

  1. I was able to work around this need by creating a dummy module object and passing that in to template.render. If there isn't enough interest, we can just leave it at that.
  2. One might also consider re-arranging the default template hierarchy so that there was a page type "above" module.html.jinja2 that doesn't require the module variable to be set, and have `module.html.jinja2' (and 'index.html.jinja2') inherit from that. This would make sense, but it would probably also be a breaking change, so perhaps not a good idea.

Additional context

You can see the site I have created at https://projectdrawdown.github.io/solutions/. I want to have the nav on all pages, even those that aren't module pages, and thus needed to do the workaround.

Also, I am willing to do this work, if it seems worthwhile.

@denised
Copy link
Contributor Author

denised commented Feb 9, 2022

FWIW, the workaround I did can be found here:
https://github.com/ProjectDrawdown/solutions/blob/develop/Documentation/autodocs/runpdoc.py

@mhils
Copy link
Member

mhils commented Feb 9, 2022

Are you aware of frame.html.jinja2? That would be the page "above" you are looking for.

If you want to keep the side bar / menu, you should be able to extend default/module.html.jinja2 if you replace all blocks that reference module. The index template is doing pretty much that, it only gets all_modules.

@denised
Copy link
Contributor Author

denised commented Feb 9, 2022

Are you aware of frame.html.jinja2? That would be the page "above" you are looking for.

Yes that is true --- I guess I was thinking of making a page-that-has-a-nav that could be the common ancestor of both module and index.html, instead of having index inherit from module. In my case, I would have added another page template that inherited from this common ancestor instead of from the module template.

If you want to keep the side bar / menu, you should be able to extend default/module.html.jinja2 if you replace all blocks that reference module. The index template is doing pretty much that, it only gets all_modules.

The one place this did not work for me was the code that cleverly figures out whether links should be local or not --- I used that in the nav, both on module pages and on non-module pages.

@mhils
Copy link
Member

mhils commented Feb 9, 2022

Yes that is true --- I guess I was thinking of making a page-that-has-a-nav that could be the common ancestor of both module and index.html, instead of having index inherit from module.

I think that is a fair point - historically the index page did not have the same sidebar layout, and when I switched it over I went the lazy way and directly inherited from module.html.jinja2. So yes, I think it does make sense to move the sidebar/layout part to frame.html.jinja2 instead and have both index and module inherit directly from there. If you want to give a stab at a PR that'd be great, otherwise I'm also happy to tackle it myself in a few weeks.

@denised
Copy link
Contributor Author

denised commented Feb 9, 2022

Honestly I'd rather leave refactoring like that to you --- I'm sure your sensitivity to fine feature details will be better than mine on this :-)

@mhils mhils changed the title Make 'module' an optional variable inside pdoc template. Move layout/sidebar into frame.html.jinja2 Feb 11, 2022
@mhils mhils self-assigned this Feb 11, 2022
@mhils mhils closed this as completed in 4a943b4 Feb 11, 2022
@mhils
Copy link
Member

mhils commented Feb 11, 2022

Thanks again for the useful feedback. I found a few hours today to properly revise the templates, the layout/sidebare now lives in frame.html.jinja2. Your use case should be properly covered now. 😃 Let me know if missed anything!

@abubelinha
Copy link

abubelinha commented Feb 18, 2022

I was using pdoc 8.3.0, with some custom template modifications using the template_directory option, after following your advice on #343
But after upgrading to 10.0.1, my docs are now pretty ugly, with some odd margin on top and left sides of the page.
(the pdoc icon has dissapeared also, but this doesn't bother me).

Is it possible that this issue is related to these problems?
In that case I would be so grateful if you can document here changes between templates of v.8.3.0 and 10.0.1 so I can see how to redo my work and adapt to new version. Otherwise I will have to go back to previous versions.

Thanks @mhils

EDIT: nevermind, I found it myself (sorry, I was expecting to find that info looking to the issues)
https://github.com/mitmproxy/pdoc/blob/main/CHANGELOG.md#2022-02-14-pdoc-1000

@denised
Copy link
Contributor Author

denised commented Feb 18, 2022

I had the same issue @abubelinha -- You probably define your own {% block nav %}. Inside that, you need to remove the outer level <nav> element, which is now part of the frame template, and leave just the contents.

@abubelinha
Copy link

Thanks a lot @denised ... now I'm on 10.0.1 🥇 😅

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

No branches or pull requests

3 participants