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

Rework typography #283

Merged
merged 10 commits into from
Aug 20, 2023
Merged

Conversation

jonathanhefner
Copy link
Member

@jonathanhefner jonathanhefner commented Aug 14, 2023

This PR is divided into several commits. Together, these commits rework the typography of rendered documentation, including font faces and sizes, margins and spacing, link appearance, and color scheme.

See the individual commits for specific changes, or see the screenshots below for the overall result. All screenshots — both before and after — were taken in Firefox on Ubuntu with DejaVu Serif (desktop @ 18px, mobile @ 16px) as the default font and DejaVu Sans Mono (desktop @ 17px, mobile @ 16px) as the default monospace font.

Before After Before (mobile) After (mobile)
before1 after1 mbefore1 mafter1
before2 after2 mbefore2 mafter2
before3 after3 mbefore3 mafter3
before4 after4 mbefore4 mafter4
before5 after5 mbefore5 mafter5
before6 after6 mbefore6 mafter6
before7 after7 mbefore7 mafter7
before8 after8 mbefore8 mafter8
before9 after9 mbefore9 mafter9

@p8
Copy link
Member

p8 commented Aug 16, 2023

Thanks for working on this @jonathanhefner !

On OSX using Firefox 116.0.2 the vertical centering of the sidebar items seems a little off.
It's also pretty hard to read for me (compared to the existing version):
image

I'm not sure what the design direction of SDoc is.
Should it be inline with the guides, the Rails homepage, or completely separate and optimized for readability?
So I can't comment on the proposed changes in that context.

@jonathanhefner
Copy link
Member Author

On OSX using Firefox 116.0.2 the vertical centering of the sidebar items seems a little off. It's also pretty hard to read for me (compared to the existing version):

The styling for the tree is unfortunately very brittle. As you noticed when working on dark mode, it uses a repeating background image and exact element heights for striping. Eventually I want to address that too, but for now I can force the tree to use the previous font. (That is what I am doing for the search results; it just didn't appear to be necessary for the tree.)

I'm not sure what the design direction of SDoc is. Should it be inline with the guides, the Rails homepage, or completely separate and optimized for readability?

The goal of this rework is to let developers read documentation using their preferred fonts and font sizes. Contrast that to the Rails homepage, where the goal is to market Rails. There is some overlap of course (for example, brand color), but there are many design decisions that are fine for marketing material but not for documentation.

With regards to the guides, in my opinion, they should also respect user font preferences. But that's outside the scope of this PR. 😉

@jonathanhefner
Copy link
Member Author

I just noticed this also fixes a styling issue with unordered lists nested in ordered lists. The former ol li rule always took precedence over the ul li rule, so an unordered list nested inside an ordered list would always be styled as an ordered list too.

Before

nested-list-before

After

nested-list-after

Technically savvy users are more likely to have a font preference
configured, and developers are more likely to have a preference for a
particular monospace font.

This commit removes the font family and base font size from the
stylesheet so that users can view the documentation with their preferred
fonts.

In particular, this stops forcing the use of the Courier font for
`<code>` elements nested in `<p>` when the Consolas and Menlo fonts are
not installed, which is commonly the case on Linux.  This also fixes
mismatching fonts for `<code>` elements nested in `<p>` versus `<code>`
elements nested in `<pre>`.
Similar to the `full_name` helper, the `short_name` helper escapes its
input and wraps the result in a `<code>` tag.  But, whereas `full_name`
will call `RDoc::CodeObject#full_name` when given an `RDoc::CodeObject`,
`short_name` will call `RDoc::CodeObject#name`.

This commit also changes the `link_to` helper to no longer escape its
input text.  Thus it can be used together with the `short_name` helper.
This factors out a `method_signature` helper that wraps method
signatures in `<code>` tags and ensures that signatures are always
HTML-escaped, including signatures from `:call-seq:` documentation.
Incidentally, this fixes a visual disparity between `:call-seq:`
signatures and normal method signatures due to the way they were wrapped
with `<b>` tags.

This commit also forces the postprocessor to use Nokogiri's `HTML5`
parser to prevent extraneous newlines from being inserted into a
signature's preformatted `<h3>` tag.
The `link.svg` file is from [Feather icons][feather] v4.29.0, and is
[licensed under the MIT license][license].

[feather]: https://feathericons.com/
[license]: https://github.com/feathericons/feather/blob/v4.29.0/LICENSE
This changes links to use the brand (i.e. banner) color.  This also
changes links to be underlined by default, except for links which have
a `<code>` element as their only child.

Additionally, this fixes hover styles so that they do not linger after
tap on mobile.
These resets are not necessary.
@jonathanhefner
Copy link
Member Author

I've updated this PR to force the tree to use its original font.

I think this PR offers a large improvement, so I am going to merge. If we decide we want to force all of the documentation to use a particular font or font size, it will be an easy change. All of the styles in this PR scale based on the root font size and the value of var(--line-height).

@jonathanhefner jonathanhefner merged commit 5a0789b into rails:main Aug 20, 2023
10 checks passed
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.

4 participants