-
Notifications
You must be signed in to change notification settings - Fork 288
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
Justify text in paragraphs #508
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @steveklabnik perhaps? I don't mind either way. |
I... am really not sure. This isn't the worst thing to do, but in my understanding, isn't considered best practice either. But I'm not 100% sure. |
Hmm, let me look a bit more into it. |
There is a few workarounds for this though. There's the The problem with
There is one correct css property which is |
@XAMPPRocky's analysis is right, and we should indeed add hyphens if we want to justify the text. Another option to do that, which I implemented in the last commit I pushed, is to use |
@pietroalbini Would it be possible to have a GitHub Pages setup with the change? I think people would be able to form a better opinion if they can read one or two articles with justified text. |
My main concern about this change is that it might be temporary. We've discussed recently about making the blog multilingual, and once we do that we have to change this code so that it can generate hyphened versions for each translation or remove it. |
We can definitely do that by generating the appropriate dictionaries. |
My main problem with that approach is that that we have to store the dictionaries in tree, and I don't really want to maintain them. If it generated/downloaded the latest ones at build time that would resolve a lot of my concern. I don't know what the correct solution for that is though, as I also know we want to keep low build times on CI. |
Deployed it in https://blog-rlo-justify-demo.netlify.com/. |
Well, we can definitely do that. The dictionaries could be then cached locally and on CI (thanks to CI's own cache). I'd prefer to avoid spending time implementing that until we have a consensus on whether we want the change or not. |
Thanks for the demo, @pietroalbini! Unfortunately the first paragraph I looked at ended up with more hyphens than I'd personally wish to see (though I can see why the layout engine would have picked them): |
To follow up on this, my disposition is to close this for now. When I initially checked this out there were a few spots with pretty bad rivers, but were pretty fixable, and it did feel nice to read. I think my main concern as I thought on it more, is that it might look out of place compared to the content on the rest of the website which will still be left aligned. I think is something we should revisit when/if we make larger changes to website and ask if we want to have content be left aligned or justified. |
I'm not sure if this is a problem in practice: the content of the blog is not the same kind of the content on the website, and I think it's fine to have different text alignmnent based on what's best for each medium. |
There was one more concern I wanted to bring up which, but forgot until @pietroalbini reminded me was how formatting can break when using inline code and on mobile devices. I've pasted screenshots below of some examples that I found when looking at it on a iPhone XS. Example 1Example 2 |
FWIW @supports (hyphens: auto) {
p /* or whatever */ {
hyphens: auto;
text-align: justify;
}
} See https://developer.mozilla.org/en-US/docs/Web/CSS/@supports. Obviously it doesn’t fix the issue with inline code samples. |
There is actually a solution to my concern now that I believe is implemented on all the platforms we support which is Screens from WWDC session where they talked about it in Safari.
|
Rebased this and added @XAMPPRocky's suggestions. The live version is @ https://blog-rlo-justify-demo.netlify.app/ |
Hmm, If you want a good example page of the bug the |
@pietroalbini what's the status of this? Do you have any interest in trying to get this landed? |
Closing this since it's been a while and there's not been movement on it. We can always reopen when there's time to work on it. |
This PR changes the styling of the blog to justify the text, which improves the look of the website and (at least for me) legibility.