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

[Docs] Update dependencies for docs #2919

Merged
merged 20 commits into from
Jan 23, 2024
Merged

[Docs] Update dependencies for docs #2919

merged 20 commits into from
Jan 23, 2024

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Dec 28, 2023

This is following the docs dependency update by https://github.com/ray-project/ray/releases/tag/ray-2.9.0

This PR also fixes the dark mode.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • Render the docs locally
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

@Michaelvll Michaelvll requested review from concretevitamin and romilbhardwaj and removed request for concretevitamin December 28, 2023 13:59
Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

Thanks, any major differences in the rendered HTML?

docs/requirements-docs.txt Show resolved Hide resolved
@Michaelvll
Copy link
Collaborator Author

Thanks, any major differences in the rendered HTML?

The new depdencies slightly improves the looking of the rendered HTML, and adds the dark mode.
This PR:
image
image

Original:
image

@romilbhardwaj
Copy link
Collaborator

Thanks @Michaelvll!
I switched to a new conda env for building and it worked fine.

Things I like:

  • Like the native dark mode support
  • Looks more modern
  • The font is slightly smaller so more of the TOC fits in the screen
  • Small indent in TOC makes it nicer and cleanly separated

Minor feedback (not necessary to fix, but nice to have):

Colors

  1. The mouseover color is a bright orange and underlines the text, seems a little jarring
image
  1. The code block font colors are harder to read than our current codeblock. See image below, left is new, right is old.
image

Is it possible to use the older color scheme?

TOC at the bottom of the landing page

Refer to image below. Left is new, right is old. The new one seems a little dense - any way to add more space there?
image

@Michaelvll
Copy link
Collaborator Author

Michaelvll commented Jan 12, 2024

Hey @romilbhardwaj, thank you for the suggestions! I have reverted the code style to the original theme, and changed the hover color to the same color as the original one. For the TOC, I could not find a easy way to increase the space. Should we merge this PR first and then figure out a way to fix that? : )

We can find the preview version here: https://skypilot.readthedocs.io/en/update-docs-dependency/

@concretevitamin
Copy link
Member

This new version looks great!

A few nits to see if there are knobs to get rid of them:

  1. Purple border around search bar: this is automatically shown when I click on the top-left search bar.
Screen Shot 2024-01-15 at 11 28 21
  1. The left-side nav bar automatically has the scroll bar displayed even when the mouse is not in the nav bar area, see pic:
Screen Shot 2024-01-15 at 11 29 16

This is not the case for the current UI.

  1. On https://skypilot.readthedocs.io/en/update-docs-dependency/, bottom of nav bar, there's an ad displayed by RTD. Is that expected? Maybe it's for preview sites? Our live docs site doesn't have the ad.

@Michaelvll
Copy link
Collaborator Author

This new version looks great!

A few nits to see if there are knobs to get rid of them:

  1. Purple border around search bar: this is automatically shown when I click on the top-left search bar.
Screen Shot 2024-01-15 at 11 28 21 2. The left-side nav bar automatically has the scroll bar displayed even when the mouse is not in the nav bar area, see pic: Screen Shot 2024-01-15 at 11 29 16 This is not the case for the current UI.
  1. On https://skypilot.readthedocs.io/en/update-docs-dependency/, bottom of nav bar, there's an ad displayed by RTD. Is that expected? Maybe it's for preview sites? Our live docs site doesn't have the ad.

Thanks for the suggestions! I fixed all of them. PTAL.

Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Michaelvll!

@Michaelvll Michaelvll merged commit f78a16e into master Jan 23, 2024
19 checks passed
@Michaelvll Michaelvll deleted the update-docs-dependency branch January 23, 2024 07:33
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.

3 participants