-
Notifications
You must be signed in to change notification settings - Fork 566
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
feat: Canvas & SVG renderers will no longer will invert characters #239
base: master
Are you sure you want to change the base?
Conversation
This is in anticipation of an update to the hanzi-writer-data & hanzi-writer-data-jp packages to fix a core issue with path layouts being horizontally flipped. BREAKING CHANGE: Users will need to update their hanzi-writer-data & hanzi-writer-data-jp character packages
Codecov ReportBase: 96.31% // Head: 96.55% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #239 +/- ##
==========================================
+ Coverage 96.31% 96.55% +0.24%
==========================================
Files 32 31 -1
Lines 1139 1161 +22
Branches 207 215 +8
==========================================
+ Hits 1097 1121 +24
+ Misses 38 36 -2
Partials 4 4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
What's the bug this is solving? The data is flipped because that's the format that makemeahanzi uses, although I'm not sure why it uses that format. The positioner corrects the flip and scales the contents to fit the window so the character should appear correctly on the screen. Is the character not rendering correctly in some scenarios? |
Not so much a bug fix, but the intention is to make it easier for users to add in their own custom characters without requiring them to write an algo to vertically flip the paths & medians (which has personally been a source of pain for adding in missing Kana and other writing systems). It should also make it easier to export the character data to SVG to be used outside the library (e.g. on React Native) without requiring scaling transforms. This change comes from a project that now utilizes these normalized characters. It certainly will warrant a major version update to prevent users from loading in the old characters. I'll be happy to close this though if you don't feel that this is part of the scope of this repo. |
What do you think about making the data parsing part of the code replaceable? So, instead of it being hardcoded to the makemeahanzi format, you could pass in a character parsing function as an option that would take in the character JSON in whatever format it's in and return a format that hanzi-writer can work with. By default it would use a parsing function for makemeahanzi, but you'd be able to make your own data format and just pass a formatting option so it would work. That way we wouldn't need to make a breaking change, while still allowing for flexibility in how the character data is structured. |
@chanind Yep, makes sense. Will add to this PR with your suggested changes soon. If I'm understanding you correctly, we'd implement a default character parser/transformer (as we're assuming this is used for |
Yeah I think that's reasonable. For cases where the data is already in the 1024x1024 and pre-flipped format, you'd still need to pass something into the |
…ata once downloaded. Note: by exposing this option, this removes the "transformY(-1)" style on SVG/Canvas renderers and instead applies a "900-Y" transform.
This is in anticipation of a major version update to the hanzi-writer-data & hanzi-writer-data-jp packages to fix a core issue with path layouts being vertically flipped. After this change, the only assumption left (in regards to character data) will be that the character's bounding box is 1024x1024.
The fix that will be applied to the characters is listed in this repository:
https://github.com/jamsch/hanzi-writer-char-fixer
BREAKING CHANGE: Users will need to update their hanzi-writer-data & hanzi-writer-data-jp character packages