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

MRG, MAINT: new website theme [skip github][skip azp][circle front] #8757

Merged
merged 37 commits into from
Feb 16, 2021

Conversation

drammock
Copy link
Member

@drammock drammock commented Jan 16, 2021

I've been poking away at the sphinx_pydata_theme off and on for the last few days. Here's a WIP pull request that makes the switch. All of you should feel free to jump in and push a commit or two if you see something I haven't fixed yet and know what the fix should be (if you don't know what the fix is, at least add it to the list below it so it doesn't get missed).

Once the circle build finishes you'll see there are some obvious remaining issues, but so far nothing that seems insurmountable. Here's a partial list:

  • some CSS properties I've tried to override but aren't working (e.g., our "sidebar" callout boxes need a bgcolor or border to distinguish them from the main text) this turned out to be an issue with dev vs stable version of the theme: the CSS color variable names had changed
  • navbar_align property in conf.py isn't working only available in dev version of theme
  • navbar has some weird spacing of items opened uneven spacing between navbar items pydata/pydata-sphinx-theme#302
  • The homepage footer is offscreen / there is a bunch of whitespace after the carousel this problem appears for @drammock locally but not on the circle build
  • what to do with the homepage sidebar boxes (currently they aren't on the side and aren't boxed)
  • any page that still has a .. contents:: directive should have it removed, since the theme includes page contents in the right sidebar
  • haven't yet tried to incorporate sphinx_multiversion, so currently our version dropdown is missing. sphinx-multiversion is for building multiple versions all at once (which I don't think we want?) and in my local tests it failed to build anything older than 0.17.

closes #8184
closes #8785

@drammock
Copy link
Member Author

here's the artifact link showing the new theme: https://24568-1301584-gh.circle-artifacts.com/0/dev/index.html

@hoechenberger

This comment has been minimized.

@cbrnr
Copy link
Contributor

cbrnr commented Jan 17, 2021

👍 for removing the gap!

@hoechenberger

This comment has been minimized.

@drammock

This comment has been minimized.

@drammock
Copy link
Member Author

are you planning to add optional Dark Mode support as well?

Not in this PR. The switch to a well supported theme seems high priority in light of how our search functionality just broke unexpectedly, but dark mode is further down the list for me.

@hoechenberger
Copy link
Member

hoechenberger commented Jan 17, 2021

This is one of the areas where CSS overrides aren't working in the way that I expect them to,

I haven't looked at the CSS, but sometimes a quick-and-dirty method is adding !important; to a style definition, have you tried that?

@agramfort
Copy link
Member

agramfort commented Jan 17, 2021 via email

@cbrnr
Copy link
Contributor

cbrnr commented Jan 17, 2021

Things that others have not mentioned:

  • The logo in the top left corner is tiny.
  • We could put a link/logo to our MNE Discourse right next to GitHub and Twitter in the top right corner.
  • It would be nice if the highlighted topics in the center image were clickable (and took you directly to the topic).
  • The button "Installation - contents" should be called "Installation".
  • The yellow background in classes (in the API section) doesn't really fit with the rest of the color scheme.
  • The dash in the copyright statement (2012-2021) should be an ndash, i.e. 2012–2021.
  • Maybe not for this PR, but I've always found it difficult to understand the difference between examples and tutorials. I assume it's probably the length (examples should be short whereas tutorials are supposed to be longer), but it makes it hard to find stuff when I have to decide between these categories as a first step. drammock edit: excellent point, but not for this PR

@hoechenberger
Copy link
Member

  • Maybe not for this PR, but I've always found it difficult to understand the difference between examples and tutorials. I assume it's probably the length (examples should be short whereas tutorials are supposed to be longer), but it makes it hard to find stuff when I have to decide between these categories as a first step.

Good thing you mention this. I always thought it’s just me. I’ve always found it confusing, too. If we’re already struggling with this, how must (new) users feel?

@drammock
Copy link
Member Author

sometimes a quick-and-dirty method is adding !important; to a style definition, have you tried that?

Yep, I was surprised that it didn't work.

put a link/logo to our MNE Discourse right next to GitHub and Twitter

Great idea

It would be nice if the highlighted topics in the center image were clickable

There is a small "try it ->" link on each one. I'll see about making the whole image clickable

"Installation - contents" should be called "Installation"

Agree, there are a few page titles that look weird as next/prev links

The yellow background in classes (in the API section) doesn't really fit with the rest of the color scheme

Not yet addressed because CSS cascading not working (see above). I think this might be a theme issue... Maybe style sheets added in conf.py are getting added before the theme default rules instead of after? Will look into it.

I've always found it difficult to understand the difference between examples and tutorials.

That is an issue I've been trying to chip away at for a while now, but progress is slow. My general intent is to make the tutorials long-form explanatory narratives that cover the most common use cases and are targeted at learners, and to make the examples into short-form how-to recipes that each demo only one thing (IMO these should be much fewer in number, and should focus on fairly specific advanced use cases like using blender to fix meshes, inverse imaging EEG with no MRI, etc).

@drammock drammock force-pushed the new-website-theme branch 2 times, most recently from 26ef265 to 7b0e57b Compare January 20, 2021 00:39
@drammock
Copy link
Member Author

latest rendering of the new website theme: https://24648-1301584-gh.circle-artifacts.com/0/dev/index.html

@agramfort
Copy link
Member

agramfort commented Jan 22, 2021 via email

@hoechenberger
Copy link
Member

hoechenberger commented Jan 22, 2021

I was also thinking that we should set the base size to 16px (currently 15px), and the ordinary text should be scaled to 1.25rem:

Screen Shot 2021-01-22 at 12 34 55

Also the text should be justified and hyphenation enabled. You'll want to use sth like:

  word-break: break-word;  /* Chrome */
  -webkit-hyphens: auto;
  -moz-hyphens: auto;
  -ms-hyphens: auto;
  -o-hyphens: auto;
  hyphens: auto;

to make it work in ~all browsers.

@hoechenberger
Copy link
Member

Also the text should be justified

Actually, the text on the front page could also be simply centered.

@larsoner
Copy link
Member

I would like to stay as close as possible to the default theme, so things like justification + hyphenation would fit better in the upstream repo. Also in this sense I agree that our devations from pandas in terms of font (I think?) and font size (at least in the navbar) should be removed. (It's possible they have custom CSS for this but I'm assuming they don't.)

@jorisvandenbossche
Copy link
Contributor

jorisvandenbossche commented Jan 22, 2021

Sorry to enter your discussion like, but I was checking this out to investigate the navbar bug report that @drammock opened (pydata/pydata-sphinx-theme#302, it's a bug in the theme, but there is an easy workaround, see the issue report). So thought I could directly give some comments/replies here as well :) (and it's cool to see that you are trying this out)

are you planning to add optional Dark Mode support as well?

There is WIP PR to add dark mode to the theme: pydata/pydata-sphinx-theme#273 (but it's certainly not yet ready, contributions are welcome!)

Maybe style sheets added in conf.py are getting added before the theme default rules instead of after? Will look into it.

Looking in the browser dev tools, the custom css gets loaded after the theme css, so that shouldn't be the problem normally (also in geopandas we have some custom css which is working there)

The logo in the top left corner is tiny

In the master version of the theme, we removed some padding around the logo, so it should appear larger now. If you still want to have it larger, there is now a --header-height variable that can be set to get this.

The yellow background in classes (in the API section) doesn't really fit with the rest of the color scheme

I think this is something we inherit from sphinx itself, and I agree it can look a bit off (also for other sites where we are using this theme, so it might be a welcome change upstream in the pydata-sphinx-theme as well)

I find the fonts a bit too small.

In the development version of the theme, we are adding CSS variables so that it should be easy to make the font size larger

our devations from pandas in terms of font (I think?) and font size (at least in the navbar) should be removed

Note that with the CSS variables (I see that for the font @drammock already used it here), it's very easy to customize this a bit, and those variables are added exactly so downstream users can do this (without needing the know the exact CSS classes etc that need to be overridden). Now, of course, you are for sure free to also follow the base theme ;), I only want to mention that those variables are meant to be overridden in a robust way.

Related to this, we are also planning to drop the custom fonts we now include in the base theme (eg OpenSans and Lato), and use the system font stack instead as default (pydata/pydata-sphinx-theme#285). But with the CSS variable it should be easy to still use another font if desired.


On another note, if you would like to keep the navbar with a dark color, like the current mne website, that should also be possible. Eg MetPy recently did this, see eg https://unidata.github.io/MetPy/latest/userguide/startingguide.html

Also, if for some pages you don't want to have a left sidebar (right now if there are no toctree entries, it only gives a strange white empty space), this is also possible to do on a page-by-page level, see https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html_sidebars (you can specify an empty list for certain pages such as the home page)

@larsoner
Copy link
Member

Thanks for the feedback @jorisvandenbossche ! To me the goal is to deviate as little as possible from the base theme. Do you know offhand how much custom CSS pandas for example uses? Based on looking at doc/source/_static/css it looks like the answer is "for most pages, little to none"

@jorisvandenbossche
Copy link
Contributor

Yes, in pandas we don't override much, but that's probably also because we initially started the theme for pandas. Eg for geopandas we override some color elements: https://github.com/geopandas/geopandas/pull/1666/files (and the PR shows how using CSS variables makes this much cleaner)
For example, currently the "highlight" color for active nagivation elements is the purple of the pandas logo. I actually have been planning to make this a more "neutral" color in the base theme, and then override this to be this purple only in the pandas docs.

Base automatically changed from master to main January 23, 2021 18:27
@larsoner
Copy link
Member

larsoner commented Jan 25, 2021

Looks like great progress, and +1k/-3.9k is awesome! Minor list of things that I think would get it MRG+1 from me:

  • Footer institutions not aligned center
  • "Download Python source code" CSS yellow does not match theme well, maybe use some named CSS theme color?
  • Version drop-box color is red, should probably match the pink of the "These are dev docs" warning box?
  • Our navbar spacing does not match pandas
  • Docs search is in the wrong place relative to pandas (theirs is upper left in meta-TOC or whatever, ours is upper right in navbar)
  • Our fonts do not match pandas
  • Tiny logo should be improved in size and readability, vs

Restructuring so that we can get the left-content-right pattern more consistent would be good, like:

https://pandas.pydata.org/docs/getting_started/index.html
https://pandas.pydata.org/docs/user_guide/dsintro.html

vs places we only have a left bar (no contents / sections in a given document):

https://24674-1301584-gh.circle-artifacts.com/0/dev/install/index.html

and places we have no left bar (not part of a toctree or whatever presumably):

https://24674-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/index.html
https://24674-1301584-gh.circle-artifacts.com/0/dev/overview/implementation.html#implementation

or neither even though we could probably have both

https://24674-1301584-gh.circle-artifacts.com/0/dev/overview/index.html

I think this would potentially be a big usability / nav benefit! I think this could be done in subsequent PRs if you want, though.

@drammock drammock mentioned this pull request Jan 25, 2021
@larsoner
Copy link
Member

+1.2k -4.1k 😍

I'll merge with master so that it builds and we can see it but feel free to push-force right over it with a rebase if you want

@drammock drammock force-pushed the new-website-theme branch 2 times, most recently from 76fced5 to e957233 Compare January 26, 2021 23:57
@drammock
Copy link
Member Author

drammock commented Jan 27, 2021

Latest rendering here. This is ready for a thorough look from the devs, and suggestions on how to deal with the remaining issues:

  1. Fonts are still small. Overriding the --font-size-base CSS variable doesn't seem to have any effect (and in general, the variables that the theme is supposed to define like --color-link don't seem to be available).
  2. On Firefox, on any page that does not need vertical scrolling, there is a lot of whitespace between the next/prev button and the footer and the footer ends up just offscreen. Doesn't happen on Chromium.
  3. The right sidebar does not appear at all on narrow screens / mobile. This is mostly OK since it only contains "page contents" links, but for the homepage it means that the "version box" doesn't appear (which is currently the only place where the "cite" link exists). FIXED (version box moved)
  4. The homepage's "direct financial support" box feels a bit more prominent than I would like. Maybe two side-by-side cards (one for the version box and one for the funders) would fix both this and the previous problem? FIXED (moved to left sidebar on large screens, hidden on small screens)
  5. The cited page is not linked from anywhere (?) and hence not discoverable. FIXED (added to TOCtree under "documentation")
  6. On desktop, the collapsible divs for different OSes don't auto-open to the detected OS. On mobile (android), the Linux one starts open and can't be closed (trying to close it, it will re-open itself immediately). FIXED
  7. It would be nice if the glossary had right-sidebar within-page links. It requires getting the glossary entries to get treated like toctree elements, or else faking that in some way (inserting hidden section headings before each glossary entry?) I consider this "nice to have, not a blocker".
  8. This theme has the same problem our old theme had: within-page anchors are often hidden under the top navbar (so if I click on a backreference link from the References section on this page, I then have to scroll up the page a bit to actually see where the in-text citation is). I will note however that this does not seem to be a problem with the within-page headings, so that's an improvement over the old theme. Probably needs an upstream fix. MOSTLY FIXED (works almost every time, no discernable pattern to failures yet)
  9. The API reference page has been split by section, so that it is navigable using the left sidebar. I think this is a usability win, but I don't love how the API landing page looks now (esp. on mobile). I think it would be possible to make the toctree hidden, and stick in a bunch of .. include:: directives, so that the landing page still had everything, but the navbar would take you to the individual pages (downside: content is duplicated, need to be careful about link targets not getting duplicated too).
  10. Another thing about the API landing page (was true in old site too): the inclusion of :py:mod:mne and the autodoc directive just below the intro text results in a self-linking mne: and then the module description "MNE software for MEG and EEG data analysis" appearing on the page. Can we suppress these somehow? Or alternatively, actually leverage autodoc to generate the API reference instead of manually sorting things by category? (bigger question possibly for a separate PR). FIXED (hidden now)

Already fixed locally, not yet pushed:

  1. Section order on the datasets overview page (someone put the "refmeg noise" section after the footbibliography)

@larsoner
Copy link
Member

The right sidebar does not appear at all on narrow screens / mobile. This is mostly OK since it only contains "page contents" links, but for the homepage it means that the "version box" doesn't appear (which is currently the only place where the "cite" link exists).

Indeed I see this as a blocker.

But this also gave me an idea -- put funding sources in the left column (or right maybe?), and give it this property that it disappears below some size (maybe sm-something). I don't think we need to show those on mobile. Basically if you solve the disappearing-right-column problem you might have the solution to the funding-sources-too-prominent problem.

The cited page is not linked from anywhere (?) and hence not discoverable.

I would put "How to cite" in bottom of the left bar of Documentation, and then add this as "Papers citing MNE" beneath that. Then both are in some toctree, and how-to-cite is both on the landing page and documentation page.

On desktop, the collapsible divs for different OSes don't auto-open to the detected OS.

This is a regression so should be fixable with appropriate CSS/JS somehow. Let me know if you want me to look, I think I had a hand in implementing the collapasing ones on main.

This theme has the same problem our old theme had: within-page anchors are often hidden under the top navbar

Did you try some variant of https://stackoverflow.com/questions/10336194/top-nav-bar-blocking-top-content-of-the-page#answer-10336221? I think I tried this with our current theme at some point but maybe using Bootstrap 4 will make it more fixable.

Can we suppress these somehow?

Can you .. container:: d-none them?

For everything else I see them as minor issues we can fix in subsequent PRs.

@drammock drammock marked this pull request as ready for review February 15, 2021 21:40
@drammock drammock changed the title WIP, MAINT: new website theme [skip github][skip azp][circle front] MRG, MAINT: new website theme [skip github][skip azp][circle front] Feb 15, 2021
@drammock
Copy link
Member Author

@cbrnr
Copy link
Contributor

cbrnr commented Feb 16, 2021

Looking very good @drammock! I like it! Only some minor points should be addressed, but this could also happen in another PR:

  • A dedicated sponsors page would be nice (see how pandas does it) - we could include a link to it in the left sidebar, and then we can discuss what to do with the right sidebar (remove, keep but link the title to the sponsors page, show the link in the left sidebar only when the right sidebar disappears).
  • Some numbers/characters in the sponsors sidebar are still different from the main text (e.g. P41, MIND, IDEX).
  • There are still duplicate links in the left sidebar and the top nav (Docs, Get help); I'd only keep them in the top navbar.
  • My preference was having all sidebars on the right; people didn't like this, and now we have two sidebars, which I think looks OK, but the problem is that the right sidebar disappears below a certain width with no replacement. This is not ideal. If we remove two links from the Version box (see previous bullet point), the sponsors sidebar could go below the version sidebar on the left.

@cbrnr
Copy link
Contributor

cbrnr commented Feb 16, 2021

One more thing that keeps annoying me: Often, searching for a particular answer on Google takes me to the dev version of our website. If I switch to the stable version, it always takes me to the homepage instead of the corresponding page in stable. Is this something that could be solved?

@larsoner larsoner mentioned this pull request Feb 16, 2021
doc/conf.py Outdated Show resolved Hide resolved
@larsoner larsoner merged commit b1d4124 into mne-tools:main Feb 16, 2021
@larsoner
Copy link
Member

Thanks @drammock !!!

@drammock drammock deleted the new-website-theme branch February 16, 2021 19:20
@hoechenberger
Copy link
Member

🎈

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

Successfully merging this pull request may close these issues.

MNE Gallery STY: Website update options
7 participants