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
Improve readability of text on site #415
Conversation
This commit includes a few notable changes to the layout in order to improve its readability: - Standard font size increased to 14pt - Line height increased to 1.5em - Width of single-column pages limited to 40em
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! 👋
Thanks for taking interest in improving the website!
As it is, there is a bunch of breakage found by navigating around.
Such a seemingly simple change is not actually trivial to implement in the current context. This is why, with the optics of doing the same kind of improvements, I knew I had to refactor the whole styles. Though as can be seen in #409, it's not going to be done.
The web design framework in use, bootstrap 2, has too many pitfalls that we have to crawl out of for what look like trivial changes.
Here's what I have quickly found out. This is likely not an exhaustive list.
This breaks the header in narrower situations
The narrow ("mobile") header has broken alignment
Iconography on the main page is wonky. At narrow width news item layout title has issues.
At many locations, often involving tables, the line-height is not applied as expected, rather preferring the px
-based one from bootstrap.
The "Get Nix" button is now misaligned with the logo, as is "Get NixOS"
Finally, while this goes into the bike shedding territory, this is a bit too much embigenning. I do agree that the font size is small in the current implementation. This gives a "computed font size" of ~18.667px. The current site has a "computed font size" of "14px", for the record.
With a previous, now cancelled, improvement attempt, I was going with a "computed font size" of "16px". In pt
, with the current CSS framework, this would be 12 pt
. Though I was going to scale it up further for extra big responsive sizes. The "16px" size is what is the most common at normal resolutions for "content" website like this one, and should also be the default of your browser. (In fact, my change relied on the default of your browser, assuming it is 16px, automatically scaling the site correctly when you ask for bigger fonts).
div.main.full-width { | ||
max-width: 40em; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full-width
name is counter-intuitive. I would have assumed, in a non-semantic way, that this means the element will use the full width of the parent container, rather than be constrained.
Hey there :) Thank you for the review! I suspected this wasn't going to be that easy after having a look at the code. Anyway, I wanted to give a try. I have seen your branch, so there likely is some duplicated effort. Before digging deeper: Do you think it's useful to do this? Are there better ways to help out with the redesign? |
Since I did the work, I'm biased to say that it was useful to do this. Though, it should be coming up in the next meeting, it would be "too much work to maintain" for the expected short time left until work for the actual redesign happens. As far as helping with the redesign, the redesign will not start from the current implementation, and it's not going to be a step-wise approach. I figure we'll soon know more what's planned and how to help. With that said, you can count on me to make sure integration of the new design implements what is necessary so it ends up fully scalable, making sure the text is always good and readable. |
Oh, I was a bit vague there. I was questioning whether me continuing to work on this PR is useful. Anyway, it is probably easier to discuss face-to-face, so I'll hold up until the next meeting, before putting any more effort into this. |
Technically speaking, it's going to be going against a core implementation detail of bootstrap 2, so way harder than it should be. As I haven't done the full work on top of bootstrap 2, I cannot say what other issues there could be. Though it does require all pages to be combed through carefully, including the manuals, which is not a trivial task. As far as "is it worth it?", it would if there was no plans for redesign. With such plans the worth is reduced. |
After reading #409 and the last meeting, I believe there are more conversations to be had, before moving on with this. So I am closing this issue. |
This commit includes a few notable changes to the layout in order to improve its readability: