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

Allow docs to be larger on large screens #1923

Merged
merged 11 commits into from Dec 19, 2021
Merged

Conversation

CaselIT
Copy link
Member

@CaselIT CaselIT commented May 28, 2021

Summary of Changes

Allow the max width of the docs to be larger for uses with a large screen. The current max body is set to 1000px.

On a large screen (2560px).
The version
image
currend read the docs
image

There is still a lot of white, but that's definitely nicer to me that the better than the current version. Personally I don't think we gain much by going wider, but it's easy to set 1200-1400px as max body width.

For reference current max is 660px

@codecov
Copy link

codecov bot commented May 28, 2021

Codecov Report

Merging #1923 (0aa7482) into master (6551ded) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1923   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        63           
  Lines         6677      6677           
  Branches      1239      1239           
=========================================
  Hits          6677      6677           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6551ded...0aa7482. Read the comment docs.

@CaselIT CaselIT requested review from kgriffs and vytas7 May 28, 2021 22:06
Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

1220px is way too large unless we also tune how fonts are scaled.

I was trying to find UX research on the subject, and while there is a lot of semi-conflicting material out there, the general conclusion is that 55-70 -- maybe slightly more -- in some cases up to 100 cpl is optimal for reading:
https://ux.stackexchange.com/questions/108801/what-is-the-best-number-of-paragraph-width-for-readability
https://www.researchgate.net/publication/234578707_Optimal_Line_Length_in_Reading--A_Literature_Review

If that is any measure, Stack Overflow seems to also use 660px (or 658px to be precise) columns for the main question and answer context.

The problem is, itself, far from new. People had been doing research on optimal layout of newspapers and books long before the computer display came.

The literature also mentions that slightly longer lines may be good for quickly scanning for information, but ~70 cpl is perfect for more thorough reading.

At the current width, our text is about 80 cpl, and about 70 cpl if you don't count whitespace.
In that light, I don't think we should increase the text paragraph width any further.
Maybe we could add some hack to allow code blocks span a little bit further to the right?

@vytas7
Copy link
Member

vytas7 commented May 29, 2021

See also: https://en.wikipedia.org/wiki/Line_length which reiterates the same points I've raised above.
Ironically, Wikipedia itself runs full width.

@CaselIT
Copy link
Member Author

CaselIT commented May 29, 2021

Note that 1200px is the complete width of the page, including the sidenav. The proposed change makes the content max width 1000px

@vytas7
Copy link
Member

vytas7 commented May 29, 2021

Another very good article: https://www.smashingmagazine.com/2014/09/balancing-line-length-font-size-responsive-web-design/
The author states, however, she found out that the limit could be slightly increased on the web from 75 to about 85.
But our docs seem to be already following that principle, with text lines averaging at 82-83 cpl.

@CaselIT
Copy link
Member Author

CaselIT commented May 31, 2021

Ok so to recap:

  • try to keep the current paragraph line length the same by adjusting the font size or maybe restricting the paragraph max length
  • make the body large enough to show ~85-88 chars in the code snippets without scrolling with the current font size.

@vytas7
Copy link
Member

vytas7 commented May 31, 2021

Yeah, plus we'll need to be remember that some snippets come inside WSGI / ASGI tab panels (as in your screenshot) and/or other nested structures such as Tips and snippets inside parameter descriptions.

That effectively uses some further amount of the precious paragraph real estate.

@CaselIT
Copy link
Member Author

CaselIT commented Jun 2, 2021

I've update the style. It's a bit larger but overall the paragraph text has similar line length, (from 660px to 700px).
With the reduced font size in the code boxes we can show 88 chars correctly. The font size of the code went from 14.15px to 13px.

Personally I think it looks better using font size of 14px for the code and 790px of max paragraph width to still show 88 chars correctly, since the code font is a bit small at 13px and I don't think it's of particular concern the number of font per line.

@vytas7 do you feel we cannot exceed the current line length on the paragraph?

also the paragraph font is 17px, and I think it's large enough already

Current version:

image

Alternative with 790px - 14px :

image

Diff of the alterinative version

diff --git a/docs/_static/custom.css b/docs/_static/custom.css
index 276bd700..d168640e 100644
--- a/docs/_static/custom.css
+++ b/docs/_static/custom.css
@@ -23,7 +23,8 @@ h1 a:hover, h2 a:hover, h3 a:hover {
 }

 div.document {
-    max-width: calc(760px + 220px); /* max body width + 220px of sidebar  */
+    /* max body width + 60px padding + 220px sidebar */
+    max-width: calc(790px + 60px + 220px);
 }

 div.footer {
@@ -165,7 +166,7 @@ pre {
 }

 .highlight pre {
-    font-size: 13px;
+    font-size: 14px;
 }

@CaselIT CaselIT requested a review from vytas7 June 2, 2021 11:45
@kgriffs kgriffs mentioned this pull request Aug 4, 2021
14 tasks
@vytas7
Copy link
Member

vytas7 commented Nov 11, 2021

Another very good article: https://www.smashingmagazine.com/2014/09/balancing-line-length-font-size-responsive-web-design/
The author states, however, she found out that the limit could be slightly increased on the web from 75 to about 85.
But our docs seem to be already following that principle, with text lines averaging at 82-83 cpl.

I still stand by my opinion that ideally I would have the line length at about 82-83 (I basically like that article), but I'm not terribly opinionated about this. We could extend to about 87-89 cpl if it is what others prefer, but my vote is on 82-83.

@kgriffs
Copy link
Member

kgriffs commented Dec 15, 2021

After comparing the three options (original, this patch, and the tweaked 790px/14px) I think we should go with the Alternative with 790px - 14px version. It does make the paragraphs a little wider than, say, medium but only by a bit and imo it's worth it to make the code sections easier to read.

@CaselIT
Copy link
Member Author

CaselIT commented Dec 15, 2021

Thanks for taking a look.

I'll update the branch with your suggestion.

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

Updated

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

@CaselIT could you also take a look into upgrading to Sphinx 4.x? From what I've seen, it does build now with my changes on some other pull request, but there are huge visual changes that probably need to be rechecked...

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

It seems to be all right. See the zip with the current version built with the latest sphinx
_build.zip

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

No, not really, I'm not saying it looks wrong, but it looks very different, for instance, some font sizes were bumped up a fair bit:

image

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

I've also reverted the docutils constraint, since it's constrainted by sphinx / sphinx-tabs etc.

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

No, not really, I'm not saying it looks wrong, but it looks very different, for instance, some font sizes were bumped up a fair bit:

image

this seems to be sqhinx or the theme doing, not a change in this pr. I can make it look like before though

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

Yes, I mean those large function name fonts are a coming from Sphinx 4.x, I noticed that when testing myself with that recently merged PR (I addressed a couple of issues preventing build on 4.x without warnings on that PR).

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

the issue is of alabaster. Sphix 4 (I suppose) uses <span> instead of <code> for these bits and the rule in alabaster is this one

tt.descname, tt.descclassname, code.descname, code.descclassname {
  font-size: 0.95em;
}

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

Another thing though, some lines now run as wide as 103-104 cpl, was it intended to increase that much? Maybe we could slightly increase the paragraph font then?

(I do like a lot that code examples avoid scrolling though 👍.)

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

let me try a couple of things

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

I've tried a couple of thinks. This is with font size 18
_build.zip

Personally I think it's too large. I prefer 17 even if the lines are a bit longer..

790px is the min to not have scrolling in examples if we want to have 14px as the code font

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

Yeah, it does feel a bit too large. At least compared to the navigation sidebar which gets too tiny in comparison.

OTOH I noticed FastAPI uses roughly the same large size (computed as 17.6px, but it can depend on the font used), and they normally stay below 100 cpl. Their code examples do use some scrolling, maybe we could also allow that in some exceptional cases provided 99% of snippets do not scroll?

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

I've tried to tweak another bit:
760px, 17.5, and made the doc snippet when using tabs a bit larger.

I think it's a good compromise
_build.zip

Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

This does look like a pretty good compromise indeed 💯
(Ideally, I would still like to reduce to 82-83 cpl and make it scale up fontsize-wise on very large screens, while designing code snippets differently, but none of us is a graphic designer, and advanced stuff like that is painful to maintain.)

@CaselIT
Copy link
Member Author

CaselIT commented Dec 18, 2021

At that point we may try to look for other templates. I recently found this one https://github.com/pradyunsg/furo that I like quite a bit

@vytas7
Copy link
Member

vytas7 commented Dec 18, 2021

Furo is looking pretty ace indeed!

@kgriffs
Copy link
Member

kgriffs commented Dec 19, 2021

I wouldn't be opposed to changing themes at some point.

Copy link
Member

@kgriffs kgriffs left a comment

Choose a reason for hiding this comment

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

Looks nice, thanks!

@kgriffs kgriffs merged commit c99972e into falconry:master Dec 19, 2021
@vytas7 vytas7 mentioned this pull request Jan 20, 2022
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.

None yet

3 participants