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

Improve readability of text on site #415

Closed
wants to merge 1 commit into from

Conversation

milibopp
Copy link
Contributor

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

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
Copy link
Member

@samueldr samueldr left a 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

image


The narrow ("mobile") header has broken alignment

image


Iconography on the main page is wonky. At narrow width news item layout title has issues.

image


At many locations, often involving tables, the line-height is not applied as expected, rather preferring the px-based one from bootstrap.

image


The "Get Nix" button is now misaligned with the logo, as is "Get NixOS"

image
image


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).

Comment on lines +192 to +194
div.main.full-width {
max-width: 40em;
}
Copy link
Member

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.

@milibopp
Copy link
Contributor Author

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?

@samueldr
Copy link
Member

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.

@milibopp
Copy link
Contributor Author

milibopp commented Apr 26, 2020

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.

@samueldr
Copy link
Member

samueldr commented Apr 27, 2020

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.

@milibopp
Copy link
Contributor Author

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.

@milibopp milibopp closed this Apr 30, 2020
@milibopp milibopp deleted the readability branch April 30, 2020 11:08
@milibopp milibopp restored the readability branch April 30, 2020 11:08
@milibopp milibopp deleted the readability branch May 11, 2020 12:11
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

2 participants