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

use official pydata sphinx theme #10971

Merged
merged 17 commits into from Apr 4, 2021
Merged

Conversation

bryevdv
Copy link
Member

@bryevdv bryevdv commented Feb 28, 2021

cc @tcmetzger

This is an early stab at removing our custom vendored version of the "pydata sphinx theme" for the official package. Unfortunately, I would say the first results are a bit wider off the mark than I had wished for. I am hoping @jorisvandenbossche or @choldgraf might be able to help shed some light or offer some pointers. I've outlined the main issues so far

Issues that aren't just styling

sidebars do not float

In the pandas docs and our old docs, the sidebars float as the central area is scrolled, affording constant navigation menus. When I build with the latest 0.4.3 version of the pydata theme, both the left and right sidebars scroll off the page:

Screen Shot 2021-02-27 at 10 17 02 PM

At this point we don't have any custom CSS or JS at all so I am not sure what could be interfering.

scroll spy is not functions

Potentially related to the above comment. The "scrollspy" functionality to highlight the current section in the view port is simply not working at all.

Probably just styling

index page too narrow

We want both sidebars to be removed on the index page. Otherwise, we get this:

Screen Shot 2021-02-27 at 10 20 14 PM

This also breaks the images at the bottom:

Screen Shot 2021-02-27 at 10 22 05 PM

Gallery page too narrow

Same story as above, we have not historically had sidebars on the gallery, because we want to fill all available space with gallery tiles:

Screen Shot 2021-02-27 at 10 22 43 PM

Version selector is gone

We will need to figure out how to inject our custom page JS that adds our version selector to the top nav bar

External link fa icons off center

Screen Shot 2021-02-27 at 10 24 49 PM

Releases page nav is badly formatted

Screen Shot 2021-02-27 at 10 26 17 PM

Gallery detail pages badly formatted

Screen Shot 2021-02-27 at 10 27 43 PM

@jorisvandenbossche
Copy link

@bryevdv cool, thanks for trying this out!

First, I would recommend to directly use the latest master version (you can still pin it to a specific commit to use a "stable" version) until the next release, since there are quite some changes since the last release that can be useful, especially regarding styling (CSS variables).

Eg those variables should make it easy to use the custom colors you were using before.

In the pandas docs and our old docs, the sidebars float as the central area is scrolled, affording constant navigation menus. When I build with the latest 0.4.3 version of the pydata theme, both the left and right sidebars scroll off the page:

That is strange, especially if you didn't include any custom adaptations for now, since many other sites are using the theme and don't have this problem.

Do you have a preview link to check out / investigate? (eg do you build / host the docs on PRs?)

@bryevdv
Copy link
Member Author

bryevdv commented Feb 28, 2021

@jorisvandenbossche Thanks for chiming in!

First, I would recommend to directly use the latest master version

I did seem to get a build error w/ master but there's alot in motion so I will try again and report back.

Do you have a preview link to check out / investigate? (eg do you build / host the docs on PRs?)

We can host test docs, but it's a manual process, I will upload a some later tonight when I am back in front of a keyboard, and leave a comment with the URL!

@choldgraf
Copy link

@bryevdv regarding the scrolling, I have found before that this kind of thing happens if bootstrap is loaded two times on the page. Could it be that you have bootstrap JS or CSS loaded both by the pydata theme and by some other thing?

@bryevdv
Copy link
Member Author

bryevdv commented Feb 28, 2021

@choldgraf I don't think so but I will also try to check on that later when I am back home. Thanks!

@tcmetzger
Copy link
Member

tcmetzger commented Feb 28, 2021

We are using a Sphinx extension called sphinx-panels - that one also uses bootstrap and might lead to bootstrap being loaded twice.
@bryevdv you could try deactivating sphinx_panels for testing, or just include panels_add_bootstrap_css = False in conf.py (as described here: https://sphinx-panels.readthedocs.io/en/latest/#sphinx-configuration)

Edit: the broken images at the bottom of the landing page you mention in your initial comment are also rendered inside boxes created with sphinx-panels, so there might indeed be some connection here.

@choldgraf
Copy link

@tcmetzger ah yep - that's the same challenge that I have had with sphinx-panels and adding that config value fixes it 👍

@bryevdv
Copy link
Member Author

bryevdv commented Mar 1, 2021

Setting panels_add_bootstrap_css = False solved the sidebar scrolling issues, thanks all!

Screen Shot 2021-02-28 at 6 22 38 PM

@bryevdv
Copy link
Member Author

bryevdv commented Mar 1, 2021

First, I would recommend to directly use the latest master version

@jorisvandenbossche one more question about the above. We install the theme in CI with a pip: stanza in a conda env file, is it possible to install from a repo commit using an env file?

@choldgraf
Copy link

@bryevdv i think so, try following a similar pattern to this one:

https://github.com/open-contracting/standard/blob/f7a466dd9265bb7ae6ecc17168b3de5f00496ddb/common-requirements.txt#L7

@bryevdv
Copy link
Member Author

bryevdv commented Mar 1, 2021

@jorisvandenbossche
Copy link

Yes, I am using this in pandas as well: https://github.com/pandas-dev/pandas/blob/9da3ca23850332e52d19250abbdcb7c482dc5508/environment.yml#L116 (not sure the "egg" part is needed, for pandas it seems to work without)

@tcmetzger
Copy link
Member

tcmetzger commented Mar 6, 2021

I finally had time to locally build this branch and take a look around - it looks very promising, I'm really looking forward to when we can use this with the docs!

Two thoughts at first glance:

We'll probably have to fix something with the bokeh-plot extension. For example, on /user_guide/layout.html#single-object I get quite some overlaps:
grafik

Also, it would be nice to have the right side bar menu in the reference guide as well (for example an entry for each class in a module).

All in all, this looks very promising!

@tcmetzger
Copy link
Member

tcmetzger commented Mar 6, 2021

I did some experimenting and just committed two of those experiments (feel free to revert any of those, of course):

I added some custom CSS back in (that got deleted with the old theme). This will definitely need some more fine-tuning, but it brings back some of the familiar colors - and fixes the gallery thumbnails on the start page, for example:
grafik

I also added our logo and a favicon, just to feel a little more "at home" 😀

Two questions about that, though:

  • As far as I can see, the html_logo has to be a relative path to conf.py - is there an easy way to use an absolute path again (such as https://static.bokeh.org/logos/logotype.svg)? And the alt text for the logo is just "logo" - is there a way to customize that as well (e.g. "Bokeh")?
  • There is only one config varable for a favicon - is there a simple way to define several favicon sizes rel="apple-touch-icon" sizes="180x180", rel="icon" type="image/png" sizes="32x32", etc)?

The OpenGraph extension seems to be working well though, all the metadata seems to be right where it should!

@bryevdv
Copy link
Member Author

bryevdv commented Mar 6, 2021

As far as I can see, the html_logo has to be a relative path to conf.py

The next (unreleased) version will has an option for a remote logo URL

There is only one config varable for a favicon

That I don't know, maybe @jorisvandenbossche or @choldgraf can comment? If nothing else I think we an override/customize the page template (without having to have a whole new theme or vendor anything)

@choldgraf
Copy link

Interesting point about the favicon, the pydata theme doesn’t do anything special with favicons, but I think that this would be a nice improvement.

@jorisvandenbossche
Copy link

And the alt text for the logo is just "logo" - is there a way to customize that as well (e.g. "Bokeh")?

That's currently hard-coded, I see: https://github.com/pydata/pydata-sphinx-theme/blob/93b5115834435c995138da4be9ad7bbcfbca8127/pydata_sphinx_theme/docs-navbar.html#L16
But that's certainly something we could make configurable if desired.

For the favicon size, that's indeed standard sphinx behaviour at the moment (that's inserted right now by inheriting from the basic sphinx template). There seems to be an issue about it over there: sphinx-doc/sphinx#7600
But short term, that could indeed something to enable with the theme itself.

Feel free to open issues/PRs about this at https://github.com/pydata/pydata-sphinx-theme!

@jorisvandenbossche
Copy link

it brings back some of the familiar colors

See also https://github.com/pydata/pydata-sphinx-theme/blob/master/pydata_sphinx_theme/static/css/theme.css for an overview of the available variables. For example, to get the different flavor of purple as currently used in the bokeh docs for highlighting the active title in the right TOC sidebar, that should be possible by setting a single variable right now (color-active-navigation)

@bryevdv
Copy link
Member Author

bryevdv commented Mar 8, 2021

There's actually quite a lot of extra header material in our other sites:

<link rel="apple-touch-icon" sizes="180x180" href="https://static.bokeh.org/favicon/apple-touch-icon.png">
<link rel="icon" type="image/png" sizes="32x32" href="https://static.bokeh.org/favicon/favicon-32x32.png">
<link rel="icon" type="image/png" sizes="16x16" href="https://static.bokeh.org/favicon/favicon-16x16.png">
<link rel="manifest" href="https://static.bokeh.org/favicon/site.webmanifest">
<link rel="mask-icon" href="https://static.bokeh.org/favicon/safari-pinned-tab.svg" color="#5bbad5">
<meta name="msapplication-TileColor" content="#da532c">
<meta name="theme-color" content="#ffffff">
<meta property="og:title" content="Bokeh" />
<meta property="og:type" content="website" />
<meta property="og:url" content="https://bokeh.org/" />
<meta property="og:description" content="Bokeh is a Python-based visualization library, capable of building plots from simple charts to interactive dashboards.">
<meta name="image" property="og:image" content="http://static.bokeh.org/og/logotype-on-hex.png">
<meta property="og:image:alt" content="Bokeh logo on hex tiles" />
<meta name="twitter:card" content="summary_large_image" />
<meta property="twitter:site" content="@bokeh" />

Which is why I thought it might be simpler just to extend the page template. IIRC is is possible to point to a new template that inherits from an existing one without creating a whole new theme? (Or am I mis-remembering that)

Edit: I guess the OG elts might come from the og extension (@tcmetzger would know) but AFAIK the rest was hand applied in the vendored version

@tcmetzger
Copy link
Member

tcmetzger commented Mar 8, 2021

@bryevdv Yes, all the og (and twitter) meta tags are generated by the OpenGraph extension. As far as I can see, this extension works well with the new theme.

@bryevdv
Copy link
Member Author

bryevdv commented Mar 12, 2021

@jorisvandenbossche trying out new 0.5 version and getting exceptions:

  File "/Users/bryan/anaconda/envs/dev/lib/python3.8/site-packages/pydata_sphinx_theme/__init__.py", line 110, in generate_toc_html
    add_header_level_recursive(soup.find("ul"), 1)
  File "/Users/bryan/anaconda/envs/dev/lib/python3.8/site-packages/pydata_sphinx_theme/__init__.py", line 103, in add_header_level_recursive
    ul["class"] = ul.get("class", []) + ["visible"]
AttributeError: 'NoneType' object has no attribute 'get'

New version does not like files that do not have headers, it seems, e.g. just:

.. toctree::
   :maxdepth: 2
   :glob:

   property/*

Is this something for us to adjust to or should I file a bug report?

@bryevdv
Copy link
Member Author

bryevdv commented Mar 12, 2021

@jorisvandenbossche also I only just realized that I think I have misunderstood what

html_theme_options = {
    "logo_link": "<other page or external link>"
}

is for. I thought it was a way to link to a remote logo file, but I think that's incorrect and that it only controls where the logo links to. Is there currently any way to use remote URLs for logos? Our logos are all on static.bokeh.org

@bryevdv bryevdv force-pushed the bryanv/10686_pydata_sphinx_theme branch from 35cf868 to 174cf1f Compare March 12, 2021 02:43
@bryevdv
Copy link
Member Author

bryevdv commented Mar 12, 2021

@tcmetzger I have pushed some fixups to build with the latest release of the theme. I also included a template override for docs-navbar.html to load our SVG logo directly from the static site:

Screen Shot 2021-03-11 at 6 45 48 PM

In this case, I had to wholesale override the jinja template. But if we want to make mods to other templates and they have relevant {% block %} sections, then we can probably get away with merely extending the existing theme template rather than replacing it. In general I am fine with customizations that are confined to the custom templates dir. My main goal is to delete all of our python code for the custom theme. For reference the templates that can be overriden are here:

https://github.com/pydata/pydata-sphinx-theme/tree/master/pydata_sphinx_theme

Edit: I'd also love to push any additional customization options back upstream and reduce the need for template customizations over time.

@bryevdv
Copy link
Member Author

bryevdv commented Mar 12, 2021

@tcmetzger I've pushed some more changes to add our custom footer that matches our other web properties (sort of... still WIP) and also added a custom.css that can be used to style it or anything else.

cc @mattpap we could probably use your input on the sizing mode example in the the layout chapter at some point.

@tcmetzger
Copy link
Member

@bryevdv I committed some updates to fix the vertical spacing and to also add the favicons back in (with layout.html)

@tcmetzger
Copy link
Member

@jorisvandenbossche Following up on adding the favicons to the theme for this pr, I have opened pydata/pydata-sphinx-theme#359 to add support for customized favicons to the theme itself.

@bryevdv
Copy link
Member Author

bryevdv commented Mar 28, 2021

@tcmetzger what is the minimum remaining work you want to see done on this before merging (i.e. if we wanted to do more work before 2.4 but were not able to, it would still be "ok")?

@tcmetzger
Copy link
Member

@bryevdv I think we'll need to figure out the issue with removing the sidebars for gallery, gallery/**, and index. And then maybe just spend some time clicking and searching around to see if everything words. Looks really good so far, though!

Less pressing but nice to have would be going through all the colors and bring them into the Bokeh color scheme (e.g. headline text color, or colors for things like .. note::).

@tcmetzger
Copy link
Member

@bryevdv My most recent commit makes the index and all gallery pages full width again:

grafik

In conf.py we needed to provide a path, not just a filename ("docs/gallery/**" instead of "gallery/**") to turn off sidebars. But I also changed layout.html to make things truely full width.

@tcmetzger
Copy link
Member

@bryevdv I customized the theme's colors in custom.css to match the Bokeh palette and colors from the current docs. I'm pretty happy with how it looks now!

Could you build and deploy this branch somewhere (https://docs.bokeh.org/en/test/ ?) so that we could do some more testing?

@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

@tcmetzger I have deployed to http://docs.bokeh.org/en/test using 2.3.1dev1 (there are no 2.4 dev builds yet)

@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

Things look really good to me FWIW, I would be 👍 to merge and make folllow-on issues

@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

@tcmetzger I pushed a small change to the ref guide model rendering that adds a section header above each model. This gets the models into the right nav menu:

Screen Shot 2021-04-03 at 5 40 33 PM

Two comments:

  • I don't love the actual inline extra section header. Maybe we can get rid of it, or maybe it's not so bad.
  • This does not yet help things like Figure that has a a gazillion methods. That will required a different approach

@tcmetzger
Copy link
Member

@bryevdv Thanks, that looks great! I'm OK with having a headline and the inline headline - this adds some good structure, and having the right hand navigation ist very helpful. Yes, we should probably improve this for things like Figure, but that could indeed be a follow up.

@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

FYI I think we will have to use something like autoclasstoc for now, because creating an actual TOC for methods is still an open issue in sphinx. I have a POC but adding it is currently slow page load (esp bokeh.plotting) with 2.3 and also needs some other tweaks, so definitely a follow on.

@tcmetzger if you agree this can be merged as-is please go ahead and merges at your earliest

@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

@tcmetzger Supposedly the CI issue is fixed but we need to kick off a build to test it, and I have some ref guide improvements I am itching to start. I am going to go ahead and merge this now and we can make follow-on issues as needed. Some things I know offhand:

  • The releases page is pretty ugly
    • the RHS navbar should probably only go one-level deep (versions) on this one page
    • The collapsible code blocks for hashes seems to not be collapsed/-ible at all any more

@bryevdv bryevdv merged commit db36cd4 into branch-2.4 Apr 4, 2021
@bryevdv bryevdv deleted the bryanv/10686_pydata_sphinx_theme branch April 4, 2021 19:16
@bryevdv
Copy link
Member Author

bryevdv commented Apr 4, 2021

@choldgraf @jorisvandenbossche @tcmetzger anyone have any ideas for quick workarounds to limit the max TOC depth just on a single page? The new default of 2 that is great in most places is not great for the releases page:

Screen Shot 2021-04-04 at 4 08 10 PM

I tried adding e.g. :maxepth: 1 to the page but it seemed to have no effect

@tcmetzger
Copy link
Member

@bryevdv Is this behavior maybe linked to sphinx-doc/sphinx#6033? @jorisvandenbossche opened this issue, so he might know best.

A (not very elegant) way to solve this could be to include some file wide metadata at the top of the releases page to indicate that we want only a one-level TOC (e.g. :manual_toc_level: 1) and then check this value in one of our custom html files. If a value 1 for manual_toc_level is detected, we would use some CSS (.toc-h3 {display: none;}) to deactivate the second TOC level.

@jorisvandenbossche
Copy link

Wow, the result now looks really great!
(I am going to steal some ideas (eg the wider homepage) for pandas I think ;))

We should also try to upstream some changes to avoid you need to customize the full layout.html template.

anyone have any ideas for quick workarounds to limit the max TOC depth just on a single page?

Based on a quick experiment with our demo docs, it seems that the :tocdepth: (what @tcmetzger referenced) actually works. But, with the caveat that the number refers to the "global" level (full site document tree), and not the local page (making it more difficult to find the correct number to specify). So if you want to limit the number of levels on the releases page to 1, and the page itself is 1 level down (only the navbar entry, no sidebar), you would need something like :tocdepth: 2. Except that it seems to start to count at 0, so I would try a :tocdepth: 1 maybe.

Now, the :tocdepth: is a sphinx file metadata value, and it actually trims the TOC (so it also won't expand to more levels when scrolling down the page). While our theme-specific option show_toc_level only determines the visibility.
So if you want to change the default displayed level for a single page (without trimming the TOC itself, so it can still expand when scrolling), then we could indeed add a custom file metadata field to the theme, as suggested by @tcmetzger.

Copy link

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quickly went through and add some comments / questions.

I think one edits you made in the layout.html was to support the wide page, right? (beeeb63) In the base theme, you can disable the sidebar, but then we still include some dummy (narrower) sidebar, which you removed I think? And in addition, you also removed the TOC right sidebar in those cases.
I think that would be something useful to have in the base theme as well, so we should think about how we could include that.

For the right TOC, I suppose we can do something as the left sidebar and make it smaller / remove it altogether if it's not present.


BTW, to give you an update in case you haven't seen: with the next version of the theme, you will also be able to add captions in the left sidebar + it now can collapse/expand subsections (see eg https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/subpages/subpage1.html), in case you would want to make use of it.

sphinx/source/_static/custom.css Show resolved Hide resolved
height:200px
}

a { color: #CF462A; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--pst-color-link should work for this, I think?

a { color: #CF462A; }

code {
font-family: SFMono-Regular,Menlo,Monaco,Consolas,Liberation Mono,Courier New,monospace;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should more or less match the default we have now, I think? (except we have Lucida Console instead of Courier New)


code {
font-family: SFMono-Regular,Menlo,Monaco,Consolas,Liberation Mono,Courier New,monospace;
color: rgb(33, 37, 41);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a PR to add this as a CSS variable as well (pydata/pydata-sphinx-theme#366), but I suppose that's only useful if we would add variables for all other aspects as well? (the padding, font size and background below)

--pst-icon-admonition-error: var(--pst-icon-times-circle);
--pst-icon-admonition-hint: var(--pst-icon-lightbulb);
--pst-icon-admonition-tip: var(--pst-icon-lightbulb);
--pst-icon-admonition-important: var(--pst-icon-exclamation-circle);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sidenote: I would personally recommend to not include all those variables that were not adapted compared to the base theme, then it will be easier to keep track of what is actually customized here)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @tcmetzger on this I don't have a preference

Comment on lines +14 to +19
<li class="nav-item dropdown">
<a class="nav-link dropdown-toggle" href="#" id="navbarDropdown" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
{{ VERSION }}
</a>
<div class="dropdown-menu" aria-labelledby="navbarDropdown" id="version-menu"></div>
</li>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct that only those selected lines above are added compared to the base theme template?

I think that pydata/pydata-sphinx-theme#355 should be able to help make this adaptation a bit cleaner.

sphinx/source/_templates/layout.html Show resolved Hide resolved
@bryevdv
Copy link
Member Author

bryevdv commented Apr 6, 2021

Thanks for all the great comments @jorisvandenbossche I have a follow on PR: #11130

I will try to implement these comments and simplifications there

FWIW rather than figure out how to use the global metadata in the template, I just did this which seems to work fine: 8513010

@jorisvandenbossche
Copy link

FWIW rather than figure out how to use the global metadata in the template, I just did this which seems to work fine:

If it works, that's probably good enough for now ;) But if you don't want to keep the javascript workaround long term, I would try putting :tocdepth: 1 at the top of the file and see if that works (so not :maxdepth: as you tried before)

bryevdv added a commit that referenced this pull request Dec 13, 2021
* use official pydata sphinx theme

* don't load bootstrap

* fixups for 0.5

* more style tweaks

* Update logo image alt text

* Add more custom CSS (sphinx-panels, etc.)

* version chooser, banner

* Style footer

* Remove extra forward/back buttons

* align navbar left

* Fix indentations and file end newline

* Fix footer vertical spacing

* Add favicons to extrahead

* Make index and gallery full width

* Customize font and admonition colors

* convert tabs to spaces

* add section headers for models

Co-authored-by: tcmetzger <ticon@gmx.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Bokeh docs to the latest version of pydata-sphinx-theme
4 participants