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

Logo is missing in 0.287 Documentation #22817

Closed
knu opened this issue May 24, 2024 · 7 comments · Fixed by #22887
Closed

Logo is missing in 0.287 Documentation #22817

knu opened this issue May 24, 2024 · 7 comments · Fixed by #22887
Assignees
Labels

Comments

@knu
Copy link

knu commented May 24, 2024

I see this piece of code in every HTML document under https://prestodb.io/docs/current/ (https://prestodb.io/docs/0.287/):

<a href="#" title="Presto 0.287 Documentation" class="md-nav__button md-logo">
  <img src="_static/" alt=" logo" width="48" height="48">
</a>

I looked into presto-docs/src/main/sphinx/ but couldn't figure out how this could happen.

@ZacBlanco
Copy link
Contributor

Thanks for pointing this out. My guess is this issue probably occurred due to a recent sphinx upgrade which broke the config. The logo url definition seems to be in ./presto-docs/src/main/sphinx/conf.py in the html_logo variable. The recent change moved us from sphinx 5.X to 7.X.

@steveburnett
Copy link
Contributor

Looking at an older version of the docs, for example the 0.286 documentation, I find

<a href="[#]" title="Presto 0.286 Documentation" class="md-nav__button md-logo">
        <img src="[_static/logo.png]" alt=" logo" width="48" height="48">
    </a>

The difference in the HTML output is

<img src="[_static/logo.png]"

and

<img src="_static/" 

@wanglinsong, could you look into this?

Reproduce:

https://prestodb.io/docs/current/

Screenshot 2024-05-28 at 4 00 41 PM

contrast with

https://prestodb.io/docs/0.286/

Screenshot 2024-05-28 at 4 00 52 PM

@steveburnett
Copy link
Contributor

steveburnett commented May 29, 2024

In conf.py I find
html_logo = 'images/logo.png'

In the images directory, I find logo.png.

Doing a local build, I find logo.png is copied to target/html/_static.

Examining the source of any local page in target/html, I find the reported <img src="_static/" problem:

<a href="#" title="Presto 0.288 Documentation" class="md-nav__button md-logo">
  <img src="_static/" alt=" logo" width="48" height="48">
</a>
  1. Reading the Sphinx doc for html_logo, I see "[the image file] width should therefore not exceed 200 pixels." logo.png is 402 pixels wide. I resize logo.png to 190 pixels width, new build, no luck. (Might want to resize logo.png anyway, but it doesn't solve the reported problem.)

  2. The Sphinx doc for html_logo also mentions "Also accepts the URL for the logo file." I edit conf.py to
    html_logo = 'https://github.com/prestodb/presto/blob/master/presto-docs/src/main/sphinx/images/logo.png', new build, no luck.

@steveburnett
Copy link
Contributor

Read through the Sphinx Changelog and didn't see anything in particular.

Editing my local requirements.txt and iterating up through sphinx versions from 5.x to 7.x, I found that html_logo is included in the build when sphinx==5.3.0, but is not included when I set sphinx==6.0.0 or above.

sphinx==5.3.0
Screenshot 2024-05-30 at 11 05 53 AM

sphinx==6.0.0
Screenshot 2024-05-30 at 11 06 39 AM

So a workaround is to set sphinx==5.3.0 in requirements.txt.

I'd like to find why our html_logo doesn't work with sphinx==6.0.0 or above, but this is a start.

@steveburnett
Copy link
Contributor

In local doc build, manually edited the file target/html/installation.html to change the page source from
<img src="_static/" alt=" logo" width="48" height="48">

to
<img src="_static/logo.png" alt=" logo" width="48" height="48">

then refreshed the page in the browser, but the logo doesn't appear.

@ZacBlanco
Copy link
Contributor

It looks like someone else had this issue and fixed it, but required them to modify the theme: bashtage/sphinx-material#136

The current theme we are using is outdated and no longer maintained. It may be worth looking into a more actively maintained sphinx theme for the docs site.

@steveburnett steveburnett moved this from 🆕 Unprioritized to 🏗 In progress in Presto Documentation May 31, 2024
@ZacBlanco
Copy link
Contributor

I took a stab at updating our theme: #22887

This fixes the logo issue and also uses a newer and more maintained sphinx theme for our docs.

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

Successfully merging a pull request may close this issue.

3 participants