falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.53k stars 945 forks source link

Allow docs to be larger on large screens #1923

Closed CaselIT closed 2 years ago

CaselIT commented 3 years ago

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[bot] commented 3 years ago

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.

vytas7 commented 3 years ago

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 commented 3 years ago

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

vytas7 commented 3 years ago

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 commented 3 years ago

Ok so to recap:

vytas7 commented 3 years ago

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 commented 3 years ago

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](https://user-images.githubusercontent.com/16175304/120467383-6dd82e80-c3a0-11eb-8cec-158d780be283.png)
Alternative with 790px - 14px : ![image](https://user-images.githubusercontent.com/16175304/120467985-040c5480-c3a1-11eb-99e3-14b947ea81dc.png)

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;
 }
vytas7 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

Thanks for taking a look.

I'll update the branch with your suggestion.

CaselIT commented 2 years ago

Updated

vytas7 commented 2 years ago

@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 commented 2 years ago

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

vytas7 commented 2 years ago

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 commented 2 years ago

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

CaselIT commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 :+1:.)

CaselIT commented 2 years ago

let me try a couple of things

CaselIT commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

CaselIT commented 2 years ago

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 commented 2 years ago

Furo is looking pretty ace indeed!

kgriffs commented 2 years ago

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