[doc] Add new Doxygen header, footer, and style customizations #16960
Conversation
jwnimmer-tri
added
status: single reviewer ok
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
from
32fbd7b to
f71d270
Compare
|
Known issues (since fixed):
|
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
from
f71d270 to
da22e7d
Compare
There was a problem hiding this comment.
+@sherm1 for review
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @sherm1)
There was a problem hiding this comment.
Class name links used to be blue, now they are a mix of blue and red. Is that intentional? If so, what is the distinction between the blue and red ones? Here is a sample from Cache:
Lunch checkpoint.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @sherm1)
|
I believe the color difference comes from whether the link has been previously visited or not. I thought that's what the main site drake.mit.edu/ was also doing, but now I see that things there are always red, I think? This seems like a bug in the style sheet. I'll check with the contractor. |
|
I think the red/blue thing is just the typical browser trick of different colors for links you've already visited. We could maybe quibble with the color choices... Took a casual look at some pages with which I am familiar. Big 👍 for better styling of code/fixed-with elements in running text! |
There was a problem hiding this comment.
I can confirm that in the new style the red links turn blue after I visit them. At least for me, the old style had all the links blue regardless of whether I had visited them (same Chrome browser). I'm not sure there is much value in color changing links in Doxygen, but I have no objection if that is intentional.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @sherm1)
There was a problem hiding this comment.
The new layout looks fine to me. I was surprised though at how much code is added here (1300 inscrutable lines) for what appears to be only a marginal change. IMO it looks a little better than the existing Doxygen but not 1300 lines worth.
But
Reviewed 4 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignee sherm1(platform) (waiting on @jwnimmer-tri)
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)
a discussion (no related file):
Working
Need to fix link blue/red style before merging.
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
doc/doxygen_cxx/doxygen_extra.css, line 1315 at r2 (raw file):
h2.groupheader { color: black; border-bottom: 1px solid #b5b5b5;
nit It looks like I missed some whitespace fixups. This file is not supposed to have literal tabs, nor it is supposed to have any trailing whitespace.
There was a problem hiding this comment.
Reviewed 4 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Working
Need to fix link blue/red style before merging.
I've confirmed that the red/blue distinction was unintentional. Stay tuned for the fix (so it's all red).
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
from
da22e7d to
45a064e
Compare
There was a problem hiding this comment.
(Rebase conflict fixed. Still waiting on contractors for the hyperlink style fix.)
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @sherm1)
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
2 times, most recently
from
cf0aec6 to
c2620d0
Compare
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri, @rpoyner-tri, and @sherm1)
doc/doxygen_cxx/doxygen_extra.css, line 1315 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
nit It looks like I missed some whitespace fixups. This file is not supposed to have literal tabs, nor it is supposed to have any trailing whitespace.
Done.
doc/BUILD.bazel, line 25 at r1 (raw file):
name = "defs", srcs = ["defs.py"], data = [
FYI The changes to header and foot image preview have already been merged into master, so they no longer appear in this PR.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
from
c2620d0 to
14036ff
Compare
There was a problem hiding this comment.
I worked through the CXX fix on my own. Please confirm in your local preview that the links are well-styled now. There should be no more blue links, only red. In particular please check that the navigation menus still work for you. (They are usually red background, not red foreground; in some earlier drafts, I had red foreground and background, which of course doesn't work well.)
Reviewable status:
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I've confirmed that the red/blue distinction was unintentional. Stay tuned for the fix (so it's all red).
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status:
There was a problem hiding this comment.
I can confirm that the links are red and stay that way now. I spotted another problem with this style sheet though. Formerly back-ticked code was rendered in a fixed font but otherwise was the same color as normal text. Now it is rendered in pink and looks very different than other text. Several problems with that:
- it seems more extreme than is warranted, making the text look clickable even though it is just fixed width
- worse, it now makes the formatting inconsistent because we don't require perfect consistency of back-ticking and other ways of getting fixed-width fonts don't trigger the highlighting. Here is an example:
Note that the same symbols occur nearly adjacent but in different colors. There is no corresponding difference in meaning.
My suggestion would be to continue rendering the back-ticked stuff in black. I don't think the new color scheme is adding anything and it is definitely a notch down in consistency. 
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @jwnimmer-tri)
jwnimmer-tri
force-pushed
the
doc-doxygen-style-updates
branch
from
14036ff to
fa38278
Compare
There was a problem hiding this comment.
Done. I changed <code> to match our regular paragraph text color.
For the record, it looks like the style inconsistency you noted was for <code> (pink) vs <pre> (black) blocks.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @rpoyner-tri)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @jwnimmer-tri)
There was a problem hiding this comment.
Looks great now, thanks!
Reviewed 1 of 3 files at r4, 1 of 1 files at r6, all commit messages.
Reviewable status:
Closes #16993.
Local preview is available as:
This change is