-
Notifications
You must be signed in to change notification settings - Fork 423
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
Support ligature #267
base: master
Are you sure you want to change the base?
Support ligature #267
Conversation
Any chance this functionality can get merged/implemented any time soon? |
I'm just waiting for this feature. Come ooon! |
👍 |
-moz-text-rendering: optimizeLegibility; | ||
-ms-text-rendering: optimizeLegibility; | ||
-o-text-rendering: optimizeLegibility; | ||
text-rendering: optimizeLegibility;| |
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 rule has a significant performance cost. Will this feature work without this rule?
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.
I know a performance problem of optimizeLegibility
. but, ligature fonts needs this property for rendering glyphs on browser. if you set ligature options to false, remove this properties.
lib/fontcustom/scripts/generate.py
Outdated
elif c == "8": | ||
c = "eight" | ||
elif c == "9": | ||
c = "nine" |
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.
Not a huge fan of this approach. Consider using an array.
words = ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine']
return words[c] || c
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.
I'm fixed this. Please review code.
@rosylilly thanks for your work on this, it seems the community would like this feature so it is a valuable addition. Please take a look at my PR comments and resolve the conflicts with master. |
ed48772
to
7964973
Compare
7964973
to
03db1c6
Compare
@JakeBeresford Rebased and Fixed. Please review my codes. |
@JakeBeresford Are you busy? I'm waiting for your reply. |
I'm going to up this PR. @kaizau or @JakeBeresford It sounds like it was ready to be merged. Is this project unmaintained? |
Given the last commit into master is from 3 years ago, probably, the project is unmaintained. |
refs: #21
It's just provide ligature mode to fontcustom.