search

wpilibsuite / frc-docs

Official FRC Documentation powered by Read the Docs
https://docs.wpilib.org
Other
146 stars 261 forks source link

Switch Theme To Furo #2626

Open GrahamSH-LLK opened 6 months ago

GrahamSH-LLK commented 6 months ago

Furo

image image image

jasondaming commented 6 months ago

I think @Daltz333 might be the best judge if and how this change could be made. As is there are multiple things I don't like about the current implementation:

Overall I like many of the features this brings I think we just need to have a deeper discussion on many of the "fine" points.

GrahamSH-LLK commented 5 months ago

I think @Daltz333 might be the best judge if and how this change could be made. As is there are multiple things I don't like about the current implementation:

  • The upper left logo looks bad and is far too elongated. I think a lot of effort has gone into the previous one and it should be very similar to that: image
  • Is there any way we can keep the font sizing more similar. It seems to have made many things bigger which throws some things off. For example the "FIRST Robotics Competition Control System" on the home page now is taking two line as opposed to 1.
  • On my screen it seems like it has semi centered the page. More gap on the right than left. I think I prefer left justified but not a hill I would die on maybe I am just resistant to change.
  • The rounded search box looked better to me.
  • The scroll on the left hand side only moves the topics and not the logo above. I don't think we need to see the logo the whole time.
  • It seems like the spacing and size of the items on the left is larger and you now need to scroll forever to reach the bottom.
  • Do all the links have to be underlined? That seems like it sets them even further apart from the text where as the old theme blended better.

Overall I like many of the features this brings I think we just need to have a deeper discussion on many of the "fine" points.

Check out the sidebar changes I just pushed. It looks way better to me. The rest I'd like to get some more opinions before changing, as I've heard very different takes on them from different people.

sciencewhiz commented 5 months ago

On the topic of looks, the people who's opinion matters most are @Kevin-OConnor (FiRST) and @arbessette (WPI). I suspect they're both busy until after championships.

Kevin-OConnor commented 5 months ago

Yep, pretty swamped through Champs and recovering the week after. Just set myself a calendar notification for the 29th to look at this. Sorry for the delay!

Kevin-OConnor commented 5 months ago

Overall I think this looks much better than when I first looked at it. Couple comments:

I like the "On this page" mini table of contents that's part of this. I like the larger text size for the title, I think it helps convey the hierarchy a little better, the text sizes are pretty close between titles and various heading levels in the current theme. image

GrahamSH-LLK commented 5 months ago

Overall I think this looks much better than when I first looked at it. Couple comments:

  • On my laptop screen the sidebar looks a little odd. I think it's a result of the overall content being centered, but the padding on the left that's the dark blue sidebar color stands out much more than the white/black padding on the right. I've included an image below. I'm not sure what exactly to do different though.

It doesn't look particularly weird to me on my laptop, but I can see how it would to others/on different resolutions. I'm not entirely sure how to fix this either- we could reduce the padding, but I feel like that might look worse.

  • In dark mode the green/teal headers on the homepage look a little ugly to me. I don't think this should be a blocker to merging, this is always something that could be changed later.

I can definitely change colors. Which color specifically?

I like the "On this page" mini table of contents that's part of this. I like the larger text size for the title, I think it helps convey the hierarchy a little better, the text sizes are pretty close between titles and various heading levels in the current theme.

image

GrahamSH-LLK commented 3 months ago

@Kevin-OConnor I changed the sidebar so that it takes up less space. Does that look better? I'm still unsure which teal color you mean exactly. (re:

In dark mode the green/teal headers on the homepage look a little ugly to me. I don't think this should be a blocker to merging, this is always something that could be changed later. )

Kevin-OConnor commented 3 months ago

@Kevin-OConnor I changed the sidebar so that it takes up less space. Does that look better? I'm still unsure which teal color you mean exactly.

LGTM. The colors of all the header blocks on the home page are the ones that don't seem to match the color scheme to me but I'm good with merging this and worrying about that later.

sciencewhiz commented 3 months ago

Lets wait to merge until we split stable and main (so we can fix any issues that emerge without back-porting).

GrahamSH-LLK commented 3 months ago

Before merging I would like to get somebody who understands one of the RTL language(s?) to make sure I didn't break anything in the change. I tried to keep everything looking like it did on the old version, but it is very possible that I missed something.

sciencewhiz commented 3 months ago

Before merging I would like to get somebody who understands one of the RTL language(s?) to make sure I didn't break anything in the change.

@AustinShalit @Starlight220

Starlight220 commented 3 months ago

Is there a way to build the Hebrew translation with the changes here? I can see if it's broken, but I'm not familiar enough with RTD/CSS/etc to know in advance.

GrahamSH-LLK commented 3 months ago

Is there a way to build the Hebrew translation with the changes here? I can see if it's broken, but I'm not familiar enough with RTD/CSS/etc to know in advance.

here you go: https://he.grahamsh.com/

Starlight220 commented 3 months ago

@GrahamSH-LLK thanks for the build!

Some panel buttons on the homepage lost the framed link: Screenshot_20240613-065956.png It seems to be that way with all the Hebrew buttons on that page; I'm starting to suspect that a lot got broken.

The headings sidebar opens properly, but the TOC sidebar doesn't open.

Another note (not only RTL), I'm not sure I'm a fan of the blockquoting here: Screenshot_20240613-070746.png

This might be a problem in the source itself (note the difference in capitalization of the language labels): Screenshot_20240613-071134.png

Screenshot_20240613-071118.png

GrahamSH-LLK commented 3 months ago

@GrahamSH-LLK thanks for the build!

Some panel buttons on the homepage lost the framed link: Screenshot_20240613-065956.png It seems to be that way with all the Hebrew buttons on that page; I'm starting to suspect that a lot got broken.

Believe it or not, those buttons seem to be missing on stable Hebrew too. I'm not sure if this is intentional, but it's not a regression in this PR.

The headings sidebar opens properly, but the TOC sidebar doesn't open.

That was just fixed (see https://he.grahamsh.com)

Another note (not only RTL), I'm not sure I'm a fan of the blockquoting here: Screenshot_20240613-070746.png

What do you dislike exactly?

This might be a problem in the source itself (note the difference in capitalization of the language labels): Screenshot_20240613-071134.png

Screenshot_20240613-071118.png

That must be a mistake from an earlier PR of mine where sphinx tabs were migrated. I'll send a PR to make it consistent soon.

sciencewhiz commented 3 months ago

Believe it or not, those buttons seem to be missing on stable Hebrew too. I'm not sure if this is intentional, but it's not a regression in this PR.

https://github.com/wpilibsuite/frc-docs-translations/issues/38

Starlight220 commented 3 months ago

Buttons: bummer it's an upstream limitation, but oh well.

TOC sidebar: fixed; thanks!

Changelog blockquote: I'm not entirely a fan of the seemingly-arbitrary blockquoting of nested lists. Why not keep it without blockquotes? Just seems a bit weird imo that part of the changelog is quoted and some isn't, but it's probably a matter of taste so I won't object.

Tab labels: sounds good.

Overall, LGTM!

GrahamSH-LLK commented 3 months ago

sphinx-rtd-theme should be able to be removed from pyproject.toml

removed.

sciencewhiz commented 3 months ago

Changelog blockquote: I'm not entirely a fan of the seemingly-arbitrary blockquoting of nested lists. Why not keep it without blockquotes? Just seems a bit weird imo that part of the changelog is quoted and some isn't, but it's probably a matter of taste so I won't object.

I agree, that looks weird. That doesn't happen here with nested lists: https://sphinx-themes.org/sample-sites/furo/kitchen-sink/lists/

sciencewhiz commented 3 months ago

This is why: https://github.com/pradyunsg/furo/discussions/190

Should probably find places with 4 space indents and fix, seems like it could show up many places.

sciencewhiz commented 2 months ago

There's conflicts to resolve

TheTripleV commented 2 months ago

https://frc-docs--2626.org.readthedocs.build/en/2626/docs/software/advanced-controls/introduction/tuning-flywheel.html

The interactive pid tuning is broken

rzblue commented 1 month ago

The js files are being added to the html past the content, so they're not available at the js entrypoint in the article. Wrapping the entrypoint in a load event listener fixes it e.g.

<script>
   addEventListener("load", (event) => {
      flywheel_pid = new FlywheelPIDF("flywheel_feedforward", "feedforward");
   });
</script>
sciencewhiz commented 4 weeks ago

Depends on https://github.com/pradyunsg/furo/issues/795

sciencewhiz commented 3 weeks ago

Depends on #2729