Skip to content

Unicode range #144

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

Merged
merged 5 commits into from
Feb 7, 2019
Merged

Unicode range #144

merged 5 commits into from
Feb 7, 2019

Conversation

raxbg
Copy link
Contributor

@raxbg raxbg commented Sep 24, 2018

The unicode-range property (https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/unicode-range) is currently not supported. This PR adds support for that.

@MyIntervals MyIntervals deleted a comment Nov 19, 2018
@MyIntervals MyIntervals deleted a comment Nov 19, 2018
@sabberworm
Copy link
Collaborator

I’m not quite on board with parsing these as literals. I guess a custom Value subclass UnicodeRange would be more appropriate. It could then also provide methods to check if a code point is included in the range…

@raxbg
Copy link
Contributor Author

raxbg commented Nov 20, 2018

Hm, I thought these were supposed to always be literals. Having a separate Value subclass sounds good, but honestly I am not sure when will I have time to do it. I was able to update this PR since the old code was reusable. As of now, this PR at least helps to keep the unicode ranges in the rendered CSS, otherwise these rules get removed.

It would be great if you(or anybody) can add the UnicodeRange type and complete the implementation, if not I will ask you to at least merge this PR as is and open an issue for the suggested improvements.

Thanks!

@MyIntervals MyIntervals deleted a comment Feb 5, 2019
@sabberworm
Copy link
Collaborator

I’ll merge it as-is, thanks.

@sabberworm sabberworm merged commit 57b42ba into MyIntervals:master Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants