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

fix: #5404 cjk keep up right bugfix (suggestion) - draft version #5405

Closed
wants to merge 1 commit into from

Conversation

jklee84
Copy link

@jklee84 jklee84 commented Jan 25, 2025

Resolve #5404

I noticed a long history in this PR.
While finding a better solution might be more challenging, I think the primary issue lies in mixed cases like 반포대로21길.

mapbox/mapbox-gl-js#3438

Before

스크린샷 2025-01-25 오후 10 52 07

After

스크린샷 2025-01-25 오후 10 51 40

Passed all unit tests

스크린샷 2025-01-25 오후 10 56 32

@jklee84 jklee84 changed the title fix: #5404 cjk keep up right bugfix fix: #5404 cjk keep up right bugfix (suggestion) Jan 25, 2025
@jklee84 jklee84 force-pushed the fix/text-keep-upright-cjk-bug branch from 828531f to 2a0c4f2 Compare January 25, 2025 13:54
@jklee84 jklee84 changed the title fix: #5404 cjk keep up right bugfix (suggestion) fix: #5404 cjk keep up right bugfix (suggestion) - draft version Jan 25, 2025
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

Attention: Patch coverage is 23.52941% with 13 lines in your changes missing coverage. Please review.

Project coverage is 89.57%. Comparing base (c151bf3) to head (2a0c4f2).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/util/script_detection.ts 7.14% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5405      +/-   ##
==========================================
- Coverage   91.95%   89.57%   -2.39%     
==========================================
  Files         282      282              
  Lines       38908    38923      +15     
  Branches     6830     6754      -76     
==========================================
- Hits        35779    34864     -915     
- Misses       3002     3872     +870     
- Partials      127      187      +60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -0,0 +1,159 @@
<!DOCTYPE html>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this would be better as a render test instead of an example.

@HarelM
Copy link
Collaborator

HarelM commented Jan 25, 2025

@1ec5 your review here would be greatly appreciated.

@jklee84 jklee84 force-pushed the fix/text-keep-upright-cjk-bug branch from 2a0c4f2 to 0a484a0 Compare January 25, 2025 14:24
@jklee84
Copy link
Author

jklee84 commented Jan 25, 2025

@1ec5 your review here would be greatly appreciated.

@HarelM @1ec5
My purpose was not to merge the code. If my PR can provide even a small insight, I would be deeply grateful for that. :)

@jklee84 jklee84 force-pushed the fix/text-keep-upright-cjk-bug branch from 0a484a0 to 9ca464c Compare January 25, 2025 14:32
@jklee84 jklee84 closed this Jan 25, 2025
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.

[Bug] There is an issue with text rotation when using text-keep-upright
2 participants