Use new theme?! - Githubissues

kmpaul commented 2 years ago

This is an attempt to try using the new sphinx-pythia-theme on the Foundations book.

I believe the critical missing issues for the Foundations book are dealing with the top navigation bar. Currently, I have the links in the top navbar to be the same as those on the Portal, but I don't think we actually want that. Perhaps we can discuss what they should be.

github-actions[bot] commented 2 years ago

This pull request is being automatically built with GitHub Actions and Netlify. To see the status of your deployment, click below.

🔍 Git commit SHA: c0e503b401a911e3dac6e3a2e4a8a505e25ec046 ✅ Deployment Preview URL: https://61df2604bb57fb24ba49d6a8--pythia-foundations.netlify.app

brian-rose commented 2 years ago

At first glance, it looks great! I agree that the top nav bar needs some thought.

Let's put a discussion about a "unified" nav bar across our sites on the agenda for an upcoming EWG.

kmpaul commented 2 years ago

Yes, and I think there is also room to discuss having 2 separate/different navbars but with similar-themed logos. There are many options.

brian-rose commented 2 years ago

One thing I notice in the preview is that the Foundations footer is missing.

This is where we currently list the licenses and give links to the feedback form and Contributor's guide.

kmpaul commented 2 years ago

Yeah. I think we need a place for that in the new theme.

kmpaul commented 2 years ago

@brian-rose: Can you take a look at this again? I've made some updates to the theme and I think the issues you mentioned are now fixed.

kmpaul commented 2 years ago

Most notable changes are the unified navbar along the top and the new navigation menu in the footer. Also note the GitHub and Twitter icons at the top-right of the page.

But there is also an extra_navbar text saying where the theme comes from, and I moved the extra_footer content to there to make room for the NSF acknowledgement in the extra_footer.

brian-rose commented 2 years ago

OK, I see those improvements! Definitely heading in the right direction. I really like the unified nav bar between this and https://github.com/ProjectPythia/projectpythia.github.io/pull/197

A few critiques:

The left nav bar should have a link to the Foundations landing page at the top of the TOC. (Yes I know that there is a "Foundations" link in the top nav bar, but the TOC should be self-contained). I suggest there should just be a link labeled "Pythia Foundations" right above "Preamble".
We're still missing the "rocket ship" Binder launch button for notebook pages.
The feedback form is pretty hard to find now. It's in a small link at the bottom of the bottom footer. I feel like we need to draw more attention to this if we want to solicit good feedback.
Related: this feedback form is meant for Foundations only. But now it's integrated into the theme so appears on the bottom footer of all pages, including non-Foundations pages. So maybe we could move the Feedback form link to the extra_navbar area?

kmpaul commented 2 years ago

[x] The home-page link in the left nav bar does need to be present. I'll figure out how to fix that.
[x] I'll look into the rocket ship button. Not sure why that isn't displaying.
~~We could add the feedback form to the top navbar just for the Foundations. I don't think that would be too jarring for people. ???~~
The bottom navbar content (i.e., feedback form link) is not integrated with the theme. The capability to display the nav menu on the bottom is a new feature of the theme, but the content can be changed in the _config.yml file (in the sphinx: config: html_theme_options: footer_menu: section). This can be different for the Portal or Foundations, so I think that content needs to be discussed for each site. So, we could drop the link to the feedback for on the Portal for now.

kmpaul commented 2 years ago

@brian-rose: Ok. I figured out why the rocket ship / Binder button wasn't being displayed. It turns out this is because Jupyter Book sets Sphinx configuration options (specifically Sphinx html_theme_options) and then overrides them wholesale if you use a sphinx: config: ... section in the _config.yml. Let me explain.

When you set the repository: url: ... option in _config.yml like so:

repository:
  url: https://path.to/my/repo

Jupyter Book translates this option to this:

html_theme_options = {
    'repository_url': 'https://path.to/my/repo',
}

in the Sphinx configuration.

If you then set any html_theme_options in the sphinx: config: ... section of the _config.yml, like so:

sphinx:
  config:
    html_theme_options:
      my_option: my_value

this will cause Jupyter Book to completely overwrite the Sphinx html_theme_options dictionary to be:

html_theme_options = {
    'my_option': 'my_value',
}

and the html_theme_options['repository_url'] value gets lost.

This is all because Jupyter Book overwrites the top-level Sphinx configuration options with anything in the sphinx: config: ... section of the _config.yml file. It does not update the Sphinx configuration options recursively (i.e., add/overwrite values in any dictionaries or lists).

Sigh.

So, the solution is to put all of the Sphinx configuration options (even the ones that would appear in the launch_buttons: or html: or repository: sections of the _config.yml file) in the sphinx: config: ... section of the _config.yml file.

It's not pretty, but it works. I've raised issue https://github.com/executablebooks/jupyter-book/issues/1588 to discuss a possible fix to this, but I'm not sure if it is problematic for other reasons.

brian-rose commented 2 years ago

@kmpaul that sounds like top-quality sleuthing on your part, congrats!

If the hack works, it works! I can see that we have our Launch button back now. I would just suggest putting comments in the _config.yml file to explain why things are not in standard Jupyterbook format (possibly including a link to this discussion) so any future Pythians are less confused about it.

ktyle commented 2 years ago

Great work @kmpaul ! I've successfully built the Foundations site with the new theme. Couple things:

sphinx-pythia-theme is available in conda-forge, but is listed as a pip install in environment.yml
For some reason, the footer-logo-nsf.png is not displaying.

kmpaul commented 2 years ago

Yes. It is available on conda-forge. I wasn't sure how long it would take to appear there, so I can not relied on pip for now. But we can make it a conda dependency now.

kmpaul commented 2 years ago

I'll look into the footer logo.

kmpaul commented 2 years ago

The footer logo displays on the preview. @ktyle, are you not seeing it in your local builds?

ktyle commented 2 years ago

Yes, on my local build. Other PNG/SVG images substituted in the tag work fine. I'm going to try a build on another system to see if it's isolated to the Apache httpd-served system on which I serve the jupyterbook.

kmpaul commented 2 years ago

@ktyle: You might try looking in the _static folder in your build directory (_build/html/_static/) to see if the image is there. If it is, then there is a problem with the link.

On what page were you seeing the image not found error?

-- Kevin

ktyle commented 2 years ago

@kmpaul The issue looks to be with the leading "/" that precedes _static in _config.yml. Eliminating that makes the image appear.

Any reason to place the NSF logo in _static as opposed to images/logos , as with our other logos?

kmpaul commented 2 years ago

That's odd. If you leave off the leading /, then it won't display on any pages that are in any subdirectories.

kmpaul commented 2 years ago

In other words, it needs to be an absolute path, not a relative one.

kmpaul commented 2 years ago

As for why I put it in the _static folder instead of images, it's because the NSF acknowledgement section is now in the extra_footer section of the PyData Sphinx Theme (which the Sphinx Book Theme uses, too). However, because that extra_footer is just text (i.e., can accept raw HTML), it is not processed by any of the Sphinx extensions. Hence, Sphinx doesn't know there is an image in that section to copy over to the _images folder in the build directory. So, to get it to work, you need to put it in a place where everything will get copied over, and that is what the _static folder does.

kmpaul commented 2 years ago

It wouldn't take a lot of effort to add a special footer section specifically for the NSF acknowledgement, and then we could take advantage of the Sphinx features to copy over the image. I think.

kmpaul commented 2 years ago

@ktyle: After removing the leading /, can you right click on the image in your browser and select the "display image in another tab/window" option? Then tell me the URL that appears in your browser.

kmpaul commented 2 years ago

When I do that, I see something like this: http://127.0.0.1:8080/_static/footer-logo-nsf.png.

ktyle commented 2 years ago

Ok, figured it out. The issue with the leading "/" is only problematic for my use case, where I'm actually serving the _build directory via the Apache web server. The server uses "/" as the document root. If I place a link there to where my Foundations _static directory is, then it all works. I'm not sure if we're going to have any users who would deploy Foundations in this manner, so it is probably just worth noting in case anyone actually does this.

kmpaul commented 2 years ago

Hmm. Yeah. That's kind of what I was thinking it must have been. I guess to extend you question, is there anyone (other than us) who is going to deploy the Foundations book at all?

Incidentally, I usually use the NodeJS http-server application for viewing Sphinx-built HTML sites on my laptop for development. And I use it primarily because the results are usually the same as what GitHub Pages deploys. Some people, though, just use the built-in HTTP server that comes with Python 3: python -m http.server --directory /path/to/html/. I like the npm module, and I just install it on my laptop with homebrew (like I do with pre-commit and some other "I always use this" stuff).

brian-rose commented 2 years ago

Hmm. Yeah. That's kind of what I was thinking it must have been. I guess to extend you question, is there anyone (other than us) who is going to deploy the Foundations book at all?

I would say that we are aiming to support users contributing to the Foundations book, but we are not directly supporting deploying the Foundations book elsewhere. I'm sure people will use it as an exemplar for their own JupyterBook projects, but we are not trying to document and support that ourselves.

kmpaul commented 2 years ago

@brian-rose @ktyle: I think this PR is in a reviewable state. I've fixed the top navbar links according to our discussion, and I've created a footer menu that primarily includes quick links to other places on the two sites. Let me know if you like this format and I'll open it up for review.

brian-rose commented 2 years ago

@brian-rose @ktyle: I think this PR is in a reviewable state. I've fixed the top navbar links according to our discussion, and I've created a footer menu that primarily includes quick links to other places on the two sites. Let me know if you like this format and I'll open it up for review.

I think it looks fantastic @kmpaul. I have no further nitpicks at this time :-)

ktyle commented 2 years ago

Looks good to me as well ... ready to open for review.

dcamron commented 2 years ago

Note that sphinx-pythia-theme#32 does drastically change the appearance of the theme for Jupyter book usage (eg here), moving away from the intentionally columnar look of the pydata/sphinx book themes, not just on the "single page" sections of the site. I'm not sure if that was by design @kmpaul, but it should probably be noted for anyone reviewing this!

vs

kmpaul commented 2 years ago

@dcamron How does it drastically change it? Is it a problem?

dcamron commented 2 years ago

@dcamron How does it drastically change it? Is it a problem?

Updated my previous comment!

kmpaul commented 2 years ago

@dcamron: It should only change the layout for wide high-res screens. Are you specifically saying that for normal "book pages" the narrower width pages should be preserved? I think I can do that. I just want to know specifically what you think looks best.

kmpaul commented 2 years ago

Ok. So you are referring to the normal "book pages". I can fix that.

kmpaul commented 2 years ago

Just FYI: I'm waiting on ProjectPythia/sphinx-pythia-theme#35 before creating a new release of the theme and then I will merge this, barring any additional feedback from people that requires changes.

brian-rose commented 2 years ago

Looks like this is good to merge! The failed link will be fixed when #205 is merged.

ProjectPythia / pythia-foundations

Use new theme?! #196