-
Notifications
You must be signed in to change notification settings - Fork 695
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
use font-display optional instead of swap #5334
Conversation
Did you check whether layouts break with the default font? |
We should also make sure to match the fallback font to the current font as much as possible. |
None of the pages I looked at have breaking changes. Would it be worth doing something like Percy for this?
do you mean match the spacing/weight or the declared font (or both)? I am not sure how much tweaking you are interested in having done Separately, we do not seem to be preloading or prefetching the fonts on the at any place in the repo, from what I can grep. Should that be added? |
Percy seems like overkill. It'd just be nice to have the fonts roughly matched. I've created #5335 to fallback to the system font instead. That works good enough in my opinion. You're right, we're not preloading the fonts. I'd sworn that I've added this back in the day. I'm not sure whether we should add it or not. It might block bandwidth on poor connections. |
I didn't think that a font prefetch that is optional would block, but that makes sense |
* use font-display optional instead of swap * add System font as Poppins and Noto Sans fallback Co-authored-by: Patrick Kettner <patrickkettner@gmail.com>
Is prefetch needed in case of font-display optional? I thought the browser will download the font anyway. |
sorry, no, thats not what I meant. Rather, I meant if you |
Created #5336 to track font matching. @sebastianbenz, no worries, your memory is working fine. We've had |
any chance it would make sense to preload the one font, rather than all 4
as before?
…On Wed, Feb 10, 2021 at 11:29 AM Matthias Rohmer ***@***.***> wrote:
Created #5336 <#5336> to
track font matching.
@sebastianbenz <https://github.com/sebastianbenz>, no worries, your
memory is working fine. We've had preload statements for the fonts and
removed them midst September which according to Kevin had a positive
effect: #4533 (comment)
<#4533 (comment)>.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5334 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADRUBS46MXKUNDTOIR7XVTS6KX6TANCNFSM4XLQV6ZQ>
.
--
patrick
|
fixes #5332