vercel / geist-font

https://vercel.com/font
SIL Open Font License 1.1
2.49k stars 65 forks source link

Issues spotted at GF onboarding time revision #132

Closed vv-monsalve closed 1 month ago

vv-monsalve commented 1 month ago

Font Name (Geist Sans/Geist Mono):

During the last onboarding review, my colleague @emmamarichal and I noticed some areas that need improvement.

Cyrillic

While Cyrillic is not among the base standard requirements for submitted fonts in GF, please consider making the following adjustments.

Kerning

vv-monsalve commented 1 month ago

Monospace issues

Screenshot 2024-10-04 at 16 15 16
vv-monsalve commented 1 month ago

Small adjustments

vv-monsalve commented 1 month ago

Interpolation

guidoferreyra commented 1 month ago

Oh I did the same fixes, we commited almost at the same time 🤣

vv-monsalve commented 1 month ago

Oh I did the same fixes, we commited almost at the same time 🤣

I decided to make a PR to speed up 🫠

couldn’t find evidence that those are used. Perhaps the glyphs should be not exported.

Yes, please; in that case, it is better not to export them.

vv-monsalve commented 1 month ago

@guidoferreyra Please bring the additional changes I was including in the PR: the license URL change in the font info and the adjustment to: softsign-cy and yeru-cy

Screenshot 2024-10-04 at 16 53 54 Screenshot 2024-10-04 at 16 57 47
vv-monsalve commented 1 month ago

Perhaps the glyphs should be not exported.

Please remember to make the nonworking ligatures non-export.

Screenshot 2024-10-08 at 18 02 04
guidoferreyra commented 1 month ago

Somehow I missed your comments on this issue. I will make these fixes asap.

guidoferreyra commented 1 month ago

@vv-monsalve The latest change on this branch https://github.com/vercel/geist-font/tree/%23135-fixes includes the fixes on the ligatures and yeru-cy

vv-monsalve commented 1 month ago

A couple of ligatures were missing to make it non-exportable. I've created a PR for the above branch, including them.

vv-monsalve commented 1 month ago

Anyway, the Cyrillic was not reviewed and requires a complete overhaul

We are going to delist the Cyrillic support to continue with the font's onboarding process as it is now.

We are seeing more issues regarding the Cyrillic. I'm leaving the added comments here so they can be addressed on an update of the font.

Cyrillic

guidoferreyra commented 1 month ago

This might be the typical GlyphsApp compoonent alignment that happens all the time. I can fix them but the cyrillic has other more big issues that should be adressed.