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: 🐛 text breaks on the last word #270

Merged
merged 1 commit into from Aug 10, 2022
Merged

Conversation

dima-hx
Copy link
Contributor

@dima-hx dima-hx commented Apr 19, 2022

Reduced font-size for elements because text don't fit in the blocks.

Description

Decimal font size in CSS and font-size in SVG are different.
#132

In SVG, when text is scaled up or down, browsers calculate the final size of the text (which is determined by the specified font size and the applied scale) and request a font of that computed size from the platform's font system. But if you request a font size of, say, 9 with a scale of 140%, the resulting font size of 12.6 doesn't explicitly exist in the font system, so the browser rounds the font size to 12 instead. This results in stair-step scaling of text.

Motivation and Context

Issue #132
image

Reduce font-size in clone styles method

Types of changes

  • [x ] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Enhancement (changes that improvement of current feature or performance)
  • Refactoring (changes that neither fixes a bug nor adds a feature)
  • Test Case (changes that add missing tests or correct existing tests)
  • Code style optimization (changes that do not affect the meaning of the code)
  • Docs (changes that only update documentation)
  • Chore (changes that don't modify src or test files)

Self Check before Merge

  • [x ] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [ x] I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • [ x] All new and existing tests passed.

@vivcat
Copy link
Contributor

vivcat bot commented Apr 19, 2022

👋 @dima-horror

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add graph.scale() method
  • docs: graph.getShortestPath is now available

Things that will help get your PR across the finish line:

  • Follow the TypeScript coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@vivcat
Copy link
Contributor

vivcat bot commented May 10, 2022

Hiya!
This PR has gone quiet. Spooky quiet. 👻
We get a lot of PRs, so we currently close PRs after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this PR or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this PR open!
Thanks for being a part of the Antv community! 💪💯

@vivcat vivcat bot added the stale label May 10, 2022
@dima-hx
Copy link
Contributor Author

dima-hx commented May 10, 2022

up

@vivcat vivcat bot removed the stale label May 11, 2022
@socram8888
Copy link

Can you please check if this issue is also present on Firefox?

I stumbled upon this issue myself too, and I think it doesn't have to do with font sizing, but rather a rounding error in the getComputedStyle, which seemingly doesn't happen on Firefox.

@dima-hx
Copy link
Contributor Author

dima-hx commented May 27, 2022

Can you please check if this issue is also present on Firefox?

I stumbled upon this issue myself too, and I think it doesn't have to do with font sizing, but rather a rounding error in the getComputedStyle, which seemingly doesn't happen on Firefox.

Currently I can't check master version on Firefox for issue present. But I can confirm that issue is not reproduced on Firefox on fixed version of package

@socram8888
Copy link

So I think I've triaged why this is happening.

It seems it is related to the "text-rendering" CSS property - the default for HTML seems to be optimizeQuality, while on SVG it is using optimizeSpeed, which has a different kerning settings:
Sin título

@dima-hx
Copy link
Contributor Author

dima-hx commented Jun 2, 2022

So I think I've triaged why this is happening.

It seems it is related to the "text-rendering" CSS property - the default for HTML seems to be optimizeQuality, while on SVG it is using optimizeSpeed, which has a different kerning settings

It seems to be yes, but no. When we found this issue we tried to play with text-rendering CSS option. This didn't give any results. Today I tried to reproduce this issue on original library, and I not receive this issue. Maybe Chrome updated something with SVG rendering?

Here it's an original code of example of this issue https://jsfiddle.net/9d172ntq/2/

@socram8888
Copy link

@dima-horror It's not the text-rendering, but the font-kerning. I've developed a custom font that tests it: https://orca.pet/kerning/

In Firefox, text-kerning: auto behaves exactly the same in HTML than SVG:
imagen

That is not true for Chrome/Edge:
imagen
Note how font-kerning: auto in HTML defaults to enabling all font kerning, but when rendering SVG, space kerning isn't enabled.

The workaround I'm gonna implement for our fork (https://github.com/RigoCorp/html-to-image) is to hardcode it to "normal" if using a Chromium-based engine.

@socram8888
Copy link

socram8888 commented Jun 5, 2022

I've published a fork with the aforementioned bug fixes, as well as working and passing tests and some other QOL improvements at https://www.npmjs.com/package/@rigocorp/html-to-image

This issue should be fixed there

@dima-hx
Copy link
Contributor Author

dima-hx commented Jun 7, 2022

I've published a fork with the aforementioned bug fixes, as well as working and passing tests and some other QOL improvements at https://www.npmjs.com/package/@rigocorp/html-to-image

This issue should be fixed there

Wow. Great job! 👷💪

Can you create MR to this origin repository? Nobody is here (from authors), but still a bit chance of merge exist 😊

@vivcat
Copy link
Contributor

vivcat bot commented Jun 28, 2022

Hiya!
This PR has gone quiet. Spooky quiet. 👻
We get a lot of PRs, so we currently close PRs after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this PR or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this PR open!
Thanks for being a part of the Antv community! 💪💯

@vivcat vivcat bot added the stale label Jun 28, 2022
@dima-hx
Copy link
Contributor Author

dima-hx commented Jun 28, 2022

up

@vivcat vivcat bot removed the stale label Jun 29, 2022
@vivcat
Copy link
Contributor

vivcat bot commented Jul 19, 2022

Hiya!
This PR has gone quiet. Spooky quiet. 👻
We get a lot of PRs, so we currently close PRs after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this PR or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this PR open!
Thanks for being a part of the Antv community! 💪💯

@vivcat vivcat bot added the stale label Jul 19, 2022
@bubkoo bubkoo merged commit 062c98a into bubkoo:master Aug 10, 2022
@vivcat
Copy link
Contributor

vivcat bot commented Aug 10, 2022

👋 @dima-horror Congrats on merging your first pull request! 🎉🎉🎉

github-actions bot pushed a commit that referenced this pull request Aug 11, 2022
# [1.10.0](v1.9.0...v1.10.0) (2022-08-11)

### Bug Fixes

* 🐛 cloneCSSStyle: copy transformOriginProp ([#297](#297)) ([76b978a](76b978a))
* 🐛 font format could be without qoutation ([#217](#217)) ([2a96149](2a96149))
* 🐛 set selected attribute on option to draw it ([#280](#280)) ([caf97c8](caf97c8))
* 🐛 text breaks on the last word ([#270](#270)) ([062c98a](062c98a))
* test specs ([c7a664e](c7a664e))

### Features

* ✨ add 'fetchRequestInit' option ([#210](#210)) ([c51da3a](c51da3a))
* ✨ added includeQueryParams flag ([#260](#260)) ([259d71e](259d71e))
@vivcat
Copy link
Contributor

vivcat bot commented Aug 11, 2022

🎉 This PR is included in version 1.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vivcat vivcat bot added the released label Aug 11, 2022
@ntorrey
Copy link

ntorrey commented Sep 4, 2022

@bubkoo Maybe I should open another issue instead of commenting here? In any case, it looks like I'm running in to a similar issue, and this PR doesn't seem to have fixed the issue for me. In the image below, if I change "8:30 Period 2 class" to "8:30 Period 2 study", the erratic behavior shows itself. Also the last letter is sometimes bumped to the next line as you can see at the end.

WhatsApp Image 2022-09-04 at 6 21 00 PM

@socram8888
Copy link

@ntorrey that's because this fix is incorrect - it uses a workaround that only really works with some fonts. Could you please try with the package @rigocorp/html-to-image? It's a fix that uses another approach (tweaking the CSS kerning).

I didn't bother creating a PR because I assumed this repo was dead, but if it's not I'll probably create a PR with all the improvements.

@ntorrey
Copy link

ntorrey commented Sep 5, 2022

@socram8888 Thank you! I tried it but ended up with the same result. One thing I didn't mention is that this problem doesn't happen on windows/chrome desktop, but it does happen with android/chrome. It's an Angular/Ionic pwa. The font link in my index.html is from https://fonts.googleapis.com/css2?family=Roboto:wght@300;400;500&display=swap

WhatsApp Image 2022-09-05 at 8 08 56 AM

istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
* 🐛 cloneCSSStyle: copy transformOriginProp ([bubkoo#297](bubkoo#297)) ([76b978a](bubkoo@76b978a))
* 🐛 font format could be without qoutation ([bubkoo#217](bubkoo#217)) ([2a96149](bubkoo@2a96149))
* 🐛 set selected attribute on option to draw it ([bubkoo#280](bubkoo#280)) ([caf97c8](bubkoo@caf97c8))
* 🐛 text breaks on the last word ([bubkoo#270](bubkoo#270)) ([062c98a](bubkoo@062c98a))
* test specs ([c7a664e](bubkoo@c7a664e))

* ✨ add 'fetchRequestInit' option ([bubkoo#210](bubkoo#210)) ([c51da3a](bubkoo@c51da3a))
* ✨ added includeQueryParams flag ([bubkoo#260](bubkoo#260)) ([259d71e](bubkoo@259d71e))
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
* 🐛 cloneCSSStyle: copy transformOriginProp ([bubkoo#297](bubkoo#297)) ([76b978a](bubkoo@76b978a))
* 🐛 font format could be without qoutation ([bubkoo#217](bubkoo#217)) ([2a96149](bubkoo@2a96149))
* 🐛 set selected attribute on option to draw it ([bubkoo#280](bubkoo#280)) ([caf97c8](bubkoo@caf97c8))
* 🐛 text breaks on the last word ([bubkoo#270](bubkoo#270)) ([062c98a](bubkoo@062c98a))
* test specs ([c7a664e](bubkoo@c7a664e))

* ✨ add 'fetchRequestInit' option ([bubkoo#210](bubkoo#210)) ([c51da3a](bubkoo@c51da3a))
* ✨ added includeQueryParams flag ([bubkoo#260](bubkoo#260)) ([259d71e](bubkoo@259d71e))
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
* 🐛 cloneCSSStyle: copy transformOriginProp ([bubkoo#297](bubkoo#297)) ([76b978a](bubkoo@76b978a))
* 🐛 font format could be without qoutation ([bubkoo#217](bubkoo#217)) ([2a96149](bubkoo@2a96149))
* 🐛 set selected attribute on option to draw it ([bubkoo#280](bubkoo#280)) ([caf97c8](bubkoo@caf97c8))
* 🐛 text breaks on the last word ([bubkoo#270](bubkoo#270)) ([062c98a](bubkoo@062c98a))
* test specs ([c7a664e](bubkoo@c7a664e))

* ✨ add 'fetchRequestInit' option ([bubkoo#210](bubkoo#210)) ([c51da3a](bubkoo@c51da3a))
* ✨ added includeQueryParams flag ([bubkoo#260](bubkoo#260)) ([259d71e](bubkoo@259d71e))
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
* 🐛 cloneCSSStyle: copy transformOriginProp ([bubkoo#297](bubkoo#297)) ([76b978a](bubkoo@76b978a))
* 🐛 font format could be without qoutation ([bubkoo#217](bubkoo#217)) ([2a96149](bubkoo@2a96149))
* 🐛 set selected attribute on option to draw it ([bubkoo#280](bubkoo#280)) ([caf97c8](bubkoo@caf97c8))
* 🐛 text breaks on the last word ([bubkoo#270](bubkoo#270)) ([062c98a](bubkoo@062c98a))
* test specs ([c7a664e](bubkoo@c7a664e))

* ✨ add 'fetchRequestInit' option ([bubkoo#210](bubkoo#210)) ([c51da3a](bubkoo@c51da3a))
* ✨ added includeQueryParams flag ([bubkoo#260](bubkoo#260)) ([259d71e](bubkoo@259d71e))
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
istaiti pushed a commit to inscreen/html-to-image that referenced this pull request Feb 7, 2023
* 🐛 cloneCSSStyle: copy transformOriginProp ([bubkoo#297](bubkoo#297)) ([76b978a](bubkoo@76b978a))
* 🐛 font format could be without qoutation ([bubkoo#217](bubkoo#217)) ([2a96149](bubkoo@2a96149))
* 🐛 set selected attribute on option to draw it ([bubkoo#280](bubkoo#280)) ([caf97c8](bubkoo@caf97c8))
* 🐛 text breaks on the last word ([bubkoo#270](bubkoo#270)) ([062c98a](bubkoo@062c98a))
* test specs ([c7a664e](bubkoo@c7a664e))

* ✨ add 'fetchRequestInit' option ([bubkoo#210](bubkoo#210)) ([c51da3a](bubkoo@c51da3a))
* ✨ added includeQueryParams flag ([bubkoo#260](bubkoo#260)) ([259d71e](bubkoo@259d71e))
@LIONSHI01
Copy link

try add css {white-space: nowrap} to the text element, it works to me

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

Successfully merging this pull request may close these issues.

None yet

5 participants