Closed LucaScorpion closed 1 year ago
Name | Link |
---|---|
Latest commit | e2f6b5a0308c460b3ce8486f9c0abad02d757887 |
Latest deploy log | https://app.netlify.com/sites/the-infi-way/deploys/64dcc9cce147930008f5a9f2 |
Deploy Preview | https://deploy-preview-78--the-infi-way.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
LGTM
Second that! Although I do see a few optimizations. My suggestions are based on the PDF below, generated on the preview site from Netlify, using FF on Windows:
2023-08-14-firefox-save-to-pdf-for-way.infi.nlpdf.pdf
By the way, I also ran an actual print from the printer (y'all did so too, right!? 😁) and it looks quite nice!
@jeroenheijmans Good feedback, thanks!
Is there a way to have the CSS suggest the header ("The Infi Way" and the URL) are suppressed? Or is that browser behavior that's not in our control?
Turns out, there is kind of a way. I found this can be done using the @page
rule. Using it does mean you never get the print header and footer though, so it's not much of a suggestion :sweat_smile: I don't think that's really an issue, but good to consider. I'll add it, and we can see what we think.
Tiny nitpick: the icons near the numbers are white-on-black but inverted is nicer for prints I think?
Agreed yes, will do!
Can we get the "Infi Test" fit on a single A4 page?
That would indeed be nicer, I can probably make that work by tightening some margins left and right.
The footer is printed on its own final page, but styled the way it is on the website. Can we perhaps improve that a little bit still? Maybe give it a print-only section header, not center it, and possibly even make it more into a list or "colofon" style section?
Yess good idea. I'll see what I can make of that (though if you have specific ideas, those are ofc more than welcome)
Some updates: I've addressed all the points, and I'm pretty happy with how it looks now.
One note: I didn't end up doing the header/footer hiding "trick", since that also meant I'd have to ensure the page margins were okay for every page, which felt very finicky and error-prone. I did set a @page
margin (so it's at least the same in all browsers). In Chrome this does seem to now always hide the header/footer, but not in FF.
Apart from that I wasn't really sure what to do with the footer, so for now I just put the links below each other, left aligned. Already a bit better than what it was imo, but I'm open for suggestions there.
The current result:
@jeroenheijmans What do you think? :)
Loving it!
One final tiny thing would be that I'd worry about is that the first page is quite fragile to a regression, if in the future the contents change only a small amount it will already plop off the page. Take this contrived, small, but somewhat reasonable example:
If I tweaked a bit in the devtools (and use !important
for a shortcut) then this would get the effect that I'd recommend:
.topic-icon {
margin: 0 !important;
border: none !important!;
}
.infi-test ol li::before {
border-radius: 0 !important;
}
.infi-test ol li {
margin-bottom: 0 !important;
}
Which prints like this, leaving room for future changes:
All this is a small thing and I'd be okay too with going forward with the current version if priorities or energy levels are needed elsewhere 😅
Ooh that's a great suggestion! I've updated the styles to match that, though I did leave the rounded corners for the numbering ;) That seemed a bit nicer to me. Final version for reference:
And with that I'll just merge it! :rocket:
Closes #70
A couple of small changes were required in the normal styles, to ensure elements didn't fall outside of their defined height (thus being cut in half on page breaks). I've also extracted a duplicated height value to a CSS var. Other than that all styling is contained in a
@media print
block.Note that the
-webkit-print-color-adjust
is specifically necessary, because Chrome doesn't respect theprint-color-adjust
property.