-
Notifications
You must be signed in to change notification settings - Fork 165
Add a font family overwrite for nonbreaking spaces #463
Conversation
Attempts to address a weird bug in Chrome where Hoefler text ` ` characters are rendered as wider than a standard space.
Holy guacamole, this hack is amazing! I'm mobile at the moment but this is an amazing amazing trick and I can't see why it shouldn't work. (Only thing would be to check that the width of this nbsp character set in Georgia is close enough in width to Hoefler that it looks the same) But wow I just want to give a silent emoji round of applause for this code, it's delicious! 👏👏 |
One thing to be aware of: any non breaking space insertions that get saved accidentally, or any intentional insertions by the user, will also render too wide on the front-end, so this fix might be needed there too. |
According to my test here, both render at ~15px wide (using our default font size), so I think we're good there: https://codepen.io/kjellr/pen/qQWgNp Our fallback fonts render slightly slimmer spaces (a couple pixels), so this may cause some very slight issues there. I didn't really notice while testing. However, I'll wait for someone else to confirm too though.
That's a good point. Spaces in the front-end will likely be less noticeable (the only reason this was noticed in the editor as because of the cursor jumping while typing. 😄). But once we're sure this definitely works for the editor, we can look at rolling it out there too. |
To clarify, I don't feel the width has to be perfect, if it's anything like I think it is (still mobile) then it's a substantial improvement. The bar to pass is to ensure the writing flow feels right and uninterrupted. |
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.
This is a vast improvement. We should absolutely ship this.
Quick one: maybe it should be called NonBreakingSpaceOverride
(override instead of overwrite)?
Also, I need a sanity check on this one. Does the height of the caret change ever so slightly on the space, or am I seeing things? To test, write a sentence, then wait with the caret at the end of the last letter, wait until it's visible, then press space and see if the height changes. It appears as if it becomes 1px taller. If it is, it's fine/acceptable but worth looking at a Georgia alternative that better mimics the height, separately.
But overall, this seems like a superbly clever hack that fixes the issue! Ship it!
Good suggestion. Updated. 👍
I added some guides to my screen to check this, and it doesn't appear to jump for me at least: In general, I tested this PR across a few different browsers in Windows, Mac, and iOS. Seems fine in each one. The only small issue I noticed is that while typing using Hoefler Text, descenders get cut off a little bit: This is resolved once you click out of the field, and is probably related to the line-height. I'll take a closer look on Monday and see if I can sort it out before merging this. 👍 |
Wow. Such a clever fix!!! 👍 |
I don't think it's a line height issue after all: the line height hasn't actually changed — it's 1.8 by default here, whether or not we're defining it where we are in this PR. Commenting our our line height change doesn't have any effect on the issue. Instead, my hypothesis is that this has something to do with the fact that Hoefler Text's descenders dip below those of Georgia: I checked some other potential font options (Times New Roman, Baskerville, etc.), but they all have the same issue — and their descenders all don't dip below the level that Hoefler Text's do. 😞 In general, still need to dig a little to figure that out... I'm going to focus on other work for a bit, but hope to circle back to this soon. |
I think it's an acceptable trade off, but would definitely be nice if we could find a font alternative that had sufficiently long descenders. Plan b we could look at a chrome only hack. |
One idea to fix the descenders: create a font with just one character for the non breaking space, either taken from Hoefler directly (just take the normal space) or something handcrafted, then base64 encode it (or not) and add it as the source instead of Georgia. I think this is possible. All depends on how long you want to work on this fix of course. Thinking if there might be other solutions... |
I don't see this issue in my example btw: |
I think it might be good to ship this as is and then look at additional improvements separately? If need be, I have a license to a font building tool, and can probably create a Hoefler/Georgia hybrid that only contains a space. |
Weird — I do see it in Safari 12.0, but it's only noticeable after bumping up the font size a bit, and only when you're typing (not on the text that's there originally): https://codepen.io/kjellr/pen/pQjjQR
My only hesitation is that while this fixes an issue in a single browser (Chrome), on a single operating system (Mac)... it also introduces this clipped text issue everywhere 😕. I'm not 100% sure that's a great tradeoff yet. |
Oh I see it with the j's now. Taking a look. |
webfontkit-20181107-022225.zip ☝️ This is a version of the Hoefler font, with the Georgia space character transplanted, and nothing else. Basically Ella's suggestion here #463 (comment) Can anyone try and see if this fixes the issue? |
Thanks! I pushed a commit that includes this fix. Seems to work during my testing. 👍 Could use another set of eyes to double check though. |
You could also transplant Hoefler's own space into the non breaking space's spot, which may be even cleaner. I wouldn't affect the space's ability to not break the line, the font is purely for rendering the character itself. |
But isn't the non breaking space spot the same as the space spot? This is getting into advanced territory for me. I think the following zip has a base64 encoded version, btw: |
To save us an extra request.
Added the base64 encoded version. Seems to work great. 👍 Happy to go ahead and merge, but I'll wait to get clarification on this part above:
|
We should get this in as soon as possible, the current behavior in master is highly disruptive to the writing flow. Just tested this again and I think it's good to ship and iterate on. |
Yup, this seems to do the trick for me 👍. Gonna go ahead and merge this…
@iseulde Happy to iterate on this with a new PR—especially if its cleaner :-) |
I think this needs to be done also for the frontend. Have you seen @swissspidy 's last post on his personal blog? Plenty of |
Ah great thanks. |
@jasmussen seems it was reverted? 14d240a I see those spaces in the front-end both in Chrome and Firefox. |
@allancole can you elaborate on the revert? |
When Allan made that revert, I'm not sure he realized this had been added to the front-end intentionally. We should re-add. |
Attempts to address #187: A weird bug in Chrome where Hoefler text
 
characters are rendered as wider than a standard space in the editor.Props to @iseulde 🙌