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

Update canvas-renderer.ts to fix Vertical alignment issues with text #2938

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haztheo
Copy link

@haztheo haztheo commented Aug 18, 2022

  • Bug 1 Fixes vertical alignment of text within divs - issue shown here: Text vertical alignment is off #2937

  • Now the fillText usage logically makes sense

  • Tested across Chrome, Firefox, Safari

Before:
1660838787558

After:
1660838848709

Fixes vertical alignment of text within divs
@haztheo haztheo changed the title Update canvas-renderer.ts Update canvas-renderer.ts to fix Vertical alignment issues with text Aug 18, 2022
@borisdayma borisdayma mentioned this pull request Aug 24, 2022
1 task
} else {
const letters = segmentGraphemes(text.text);
letters.reduce((left, letter) => {
this.ctx.fillText(letter, left, text.bounds.top + baseline);

this.ctx.fillText(text.text, text.bounds.left, text.bounds.top + text.bounds.height - baseline);

Choose a reason for hiding this comment

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

I am just testing this out on my current project. Should the first 2 parameters here not stay as letter and left?

@eugeneZolotarenko
Copy link

Can we merge it?

@JoaquimLey
Copy link

Hey, this was approved for close to a year now? Can we merge it?

@Sharcoux
Copy link

Can be tested in @cantoo/html2canvas

@caendesilva
Copy link

caendesilva commented Feb 3, 2024

I tried this out and for me the text got justified too much.

Latest release:

With this patch:

This patch, but after applying this suggestion https://github.com/niklasvh/html2canvas/pull/2938/files#r972962898

caendesilva added a commit to caendesilva/Windowlight that referenced this pull request Feb 3, 2024
@acurrieclark
Copy link

I have done a little more research into this, and it seems that the calculation is using the incorrect parameters. The baseline should actually be the font size in px. Will make a new PR.

@Sharcoux
Copy link

Ah, damn. @acurrieclark do you mind pushing your fix to this project too: https://github.com/cantoo-scribe/html2canvas ? This one seems a bit abandoned and I tried to merge the most promising PRs inside it

@acurrieclark
Copy link

@Sharcoux in case you didn't see, I raised this as an issue on your fork. There are a couple of possible solutions to the problem. I have found one which fixes things on my end, but it may not be the most general solution.

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.

7 participants