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

Update docs theme to sphinx-immaterial #22887

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

ZacBlanco
Copy link
Contributor

@ZacBlanco ZacBlanco commented May 31, 2024

Updates our old docs theme to to sphinx-immaterial theme. Fixes the issue with the logo and makes the site feel a little newer.

image

Fixes #22817

@steveburnett
Copy link
Contributor

Nice! Thanks for including the screenshot of the new style. I've done a local build of your branch and it looks the same as your screenshot, which is not surprising at all but nice nonetheless.

One question:
Is it possible for us to keep the "View page source / Edit this page / Create docs issue / Create project issue" links in the existing doc's right sidebar?
(Screenshot of the existing Presto Concepts for comparison with your screenshot.)
Screenshot 2024-05-31 at 4 23 21 PM

@ZacBlanco
Copy link
Contributor Author

This should work now. I also added a light/dark mode toggle which is a feature of this new theme:

image

Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local docs build.

  • The "View page source / Edit this page / Create docs issue / Create project issue" links in the existing doc's right sidebar are back as requested.
  • The light/dark mode toggle is present and it works - thanks for adding it in!

I have no other concerns. This fixes the original problem of the missing Presto logo, updates us to a currently-maintained Sphinx theme, and adds a light/dark mode feature for our readers. Thanks!

@ZacBlanco ZacBlanco marked this pull request as ready for review June 3, 2024 15:03
@ZacBlanco ZacBlanco requested a review from a team as a code owner June 3, 2024 15:03
@ZacBlanco ZacBlanco requested a review from presto-oss June 3, 2024 15:03
@ZacBlanco
Copy link
Contributor Author

@tdcmeehan would you like to look before I merge this?

@tdcmeehan tdcmeehan self-assigned this Jun 3, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Also, added auto-scrolling TOC and system media queries for
automatically picking dark/light mode color schemes.
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

Thanks @ZacBlanco !

@ZacBlanco ZacBlanco merged commit 56f22ce into prestodb:master Jun 3, 2024
57 checks passed
@steveburnett
Copy link
Contributor

steveburnett commented Jun 13, 2024

Just noticed this today, not sure when it happened.

Some of the right border tables of content for individual pages have the headings preceded with a purple "M" character.

This affects most of the pages in /functions/ and in /rest/, but I didn't find this problem elsewhere in the Presto doc set.

I'm not seeing any errors or warnings in a local docs build that would explain this.

Example screenshots from a local docs build of https://prestodb.io/docs/current/functions/conversion.html, and https://prestodb.io/docs/current/rest/node.html.
Screenshot 2024-06-13 at 2 56 58 PM
Screenshot 2024-06-13 at 2 56 47 PM

@ZacBlanco
Copy link
Contributor Author

@steveburnett That's part of the theme -- when using the ..function:: directive in the RST the theme inserts M (for "Method"). I can see about removing it if you think it's distracting

@steveburnett
Copy link
Contributor

@ZacBlanco thanks - I thought they might be a feature of the theme instead of a bug, but I wanted to ask. If you can find an easy way to disable this feature, I'd appreciate it.

@ZacBlanco ZacBlanco mentioned this pull request Jun 14, 2024
6 tasks
@wanglinsong wanglinsong mentioned this pull request Jun 25, 2024
36 tasks
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.

Logo is missing in 0.287 Documentation
3 participants