Skip to content
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

Issues spotted at GF onboarding time revision #132

Closed
2 tasks done
vv-monsalve opened this issue Oct 4, 2024 · 12 comments
Closed
2 tasks done

Issues spotted at GF onboarding time revision #132

vv-monsalve opened this issue Oct 4, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@vv-monsalve
Copy link
Contributor

vv-monsalve commented Oct 4, 2024

Font Name (Geist Sans/Geist Mono):

  • 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.

  • Please review the connection (the descender is not well attached) and the vertical position of the following glyphs ҖҚҢҲҶҗқңҳҷ:
Screenshot 2024-10-04 at 15 38 13
  • The bar could be better centered:
Screenshot 2024-10-04 at 15 41 15

Kerning

  • The Cyrillic kerning could be improved:
Screenshot 2024-10-04 at 18 07 38 Screenshot 2024-10-04 at 18 08 16 Screenshot 2024-10-04 at 18 08 22
@vv-monsalve vv-monsalve added the bug Something isn't working label Oct 4, 2024
@vv-monsalve
Copy link
Contributor Author

vv-monsalve commented Oct 4, 2024

Monospace issues

  • Most ligatures tested are working ok but

    • <---
    Screenshot 2024-10-04 at 16 03 55
    • ###
Screenshot 2024-10-04 at 16 15 16
  • Alignment issue
    The glyph could be decomposed as in the proportional Geist. It's highly suggested not to combine components with paths.
Screenshot 2024-10-04 at 15 34 17 Screenshot 2024-10-04 at 15 37 54

@vv-monsalve
Copy link
Contributor Author

vv-monsalve commented Oct 4, 2024

Small adjustments

  • Please review the currency position throughout the masters,
Screenshot 2024-10-04 at 15 30 33
  • The comma could be centered with the T stem:
Screenshot 2024-10-04 at 15 32 34 Screenshot 2024-10-04 at 15 33 03

@vv-monsalve
Copy link
Contributor Author

vv-monsalve commented Oct 4, 2024

Interpolation

  • uhungarumlaut is not using the right component
Screenshot 2024-10-04 at 17 49 11

@guidoferreyra
Copy link
Collaborator

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

  • Anyway, the cyrillic was not reviewed and requires a complete overhaul, nevertheless, I made the changes you asked.
  • The two missing coding ligatures are left out from the feature because I couldn’t find evidence that those are used. Perhaps the glyphs should be not exported.

@vv-monsalve
Copy link
Contributor Author

vv-monsalve commented Oct 4, 2024

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Collaborator

guidoferreyra commented Oct 8, 2024

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

@guidoferreyra guidoferreyra reopened this Oct 8, 2024
@guidoferreyra
Copy link
Collaborator

@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
Copy link
Contributor Author

vv-monsalve commented Oct 11, 2024

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

@vv-monsalve
Copy link
Contributor Author

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

  • uni0409: this thicker stem was not here in the previous version
Screenshot 2024-10-24 at 10 08 43
  • very small connexion issue with the components:
Screenshot 2024-10-24 at 10 09 41 Screenshot 2024-10-24 at 10 10 12
  • alignement issue:
Screenshot 2024-10-24 at 10 09 55

@guidoferreyra
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants