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

Add a fixed font for farsi #1168

Merged
merged 3 commits into from
May 29, 2024
Merged

Conversation

ciegler
Copy link
Contributor

@ciegler ciegler commented May 27, 2024

Adds vazirmatn as a fixed font for farsi as suggested in issue: #1141

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works. But since I cannot farsi, I can hardly judge if this constitutes an improvement or not ^^'

That string concat in the webpack config looks awfully brittle though, at least at a glance? How likely do you think this is to break with future updates? Anything future readers of this config should know about?

Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this! I noticed a couple things that could be improved I think. Maybe you can take another look at those. Don't hesitate to ask if my comments are unclear!

webpack.config.ts Outdated Show resolved Hide resolved
webpack.config.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise this looks good to me! For a last check: summoning @ferishili! Can you take a look at the deployment of this PR and see if everything looks alright to you? You suggested the font :)
Comparing that to the current version I certainly see a difference, i.e. the font has changed. But it's obviously a bit better to have someone who can actually understand the pretty symbols :D

@ferishili
Copy link
Contributor

That looks great, thanks for the PR.

@LukasKalbertodt
Copy link
Member

Thanks for the quick reply. Merging then!

@LukasKalbertodt LukasKalbertodt merged commit 0fc35f0 into elan-ev:master May 29, 2024
2 checks passed
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.

None yet

4 participants