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

Use local fonts #24

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Use local fonts #24

merged 1 commit into from
Jul 18, 2023

Conversation

yjose
Copy link
Member

@yjose yjose commented Jul 17, 2023

Why

fixing #23
For some reason, the library can't find fonts, and it looks like this is a common issue with the Jimp library on Windows only. To fix this issue, I copied fonts from the node_modules folder to the assets folder and it seems to work.

The solution was not yet tested on windows to make sure its working as expected.

@Pkcarreno, if you have some time, I would like if you could test this fix on Windows. You just need to install the dependencies and run pnpm start. If it works without any issues, that means it's the right fix.

@yjose yjose requested a review from SihamBen July 17, 2023 14:34
@Pkcarreno
Copy link

Pkcarreno commented Jul 17, 2023

Hey @yjose, thanks for your work on this

I did two tests
Run the library with pnpm start it works without problems, the 2 images appear and it does not show any error.

Then I installed the library in a clean project with pnpm install "https://github.com/obytes/app-icon-badge.git#fix-font-windows" and when I tried to compile it gave me the same error.

So I did one more test, I took this library in the master branch and ran pnpm start and it worked fine too, sorry for not doing this before.

Looking at it this way it doesn't seem to be an error of the library you mention, is there any difference in build time?

@yjose
Copy link
Member Author

yjose commented Jul 18, 2023

Okay, great, thank you @Pkcarreno for your feedback. I think it should work as expected after pushing a new version to npm.

@SihamBen, can you please check this fix and push a new version to npm

@SihamBen SihamBen merged commit af3c64d into master Jul 18, 2023
@Pkcarreno
Copy link

Thank you very much @yjose, I will be waiting to close the open issues when the new version is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants