Fix the issue where TextNode does not render when it is an empty string and the line-height issue in non-Firefox browsers; also, fix the misalignment of textDecorationLine at high resolutions. #3182
Conversation
… being overlooked in rendering due to being perceived as blank by trim().
…x-based browsers by modifying the x-axis calculation method. This update addresses line-height issues caused by differences in x-axis calculation methods in browsers that do not use the Firefox engine. By refining the x-axis calculation method, it ensures uniform line-height behavior across all major browsers.
…n-Firefox-based browsers by modifying the x-axis calculation method. This update addresses line-height issues caused by differences in x-axis calculation methods in browsers that do not use the Firefox engine. By refining the x-axis calculation method, it ensures uniform line-height behavior across all major browsers." This reverts commit 806f466.
…x-based browsers by modifying the x-axis calculation method. This update addresses line-height issues caused by differences in x-axis calculation methods in browsers that do not use the Firefox engine. By refining the x-axis calculation method, it ensures uniform line-height behavior across all major browsers.
…rors on high-resolution devices due to varying devicePixelRatio, corrected by using relative values of element heights.
|
Is it possible to push this on: |
Imran-imtiaz48
left a comment
There was a problem hiding this comment.
Review Summary
The changes to the CanvasRenderer.js file include updates to how text decorations are drawn on a canvas. These changes introduce a variable fillHeight to adjust the height of the text decoration lines dynamically and aim to fix issues related to high-resolution devices. The code appears to address previous comments and TODOs by streamlining the drawing process.
Key Changes
-
Variable Introduction:
- Added
var fillHeight = 1;to standardize the fill height of the text decorations.
- Added
-
Updated Drawing Logic:
- Simplified the drawing logic for
TEXT_DECORATION_LINE_UNDERLINE,TEXT_DECORATION_LINE_OVERLINE, andTEXT_DECORATION_LINE_LINE_THROUGHto use the newfillHeightvariable.
- Simplified the drawing logic for
-
Removed Redundant Code:
- Removed old calculations and TODO comments related to handling font size variations and browser inconsistencies.
Recommendations
The changes improve the readability and maintainability of the code. However, here are a few recommendations:
- Variable Naming: Consider using a more descriptive name for
fillHeight, such asdecorationLineHeight, to clarify its purpose. - Code Comments: Although some TODO comments were removed, it might be beneficial to add comments explaining why
fillHeightis used and how it resolves the previous issues. - Testing: Ensure thorough testing across different browsers and devices to confirm that the new approach consistently resolves the high-resolution device issues and maintains correct text decoration rendering.
Example with Recommendations Applied
Here's a slightly modified version with a more descriptive variable name and an added comment:
export class CanvasRenderer extends Renderer {
...
if (styles.textDecorationLine.length) {
this.ctx.fillStyle = asString(styles.textDecorationColor || styles.color);
styles.textDecorationLine.forEach((textDecorationLine) => {
var decorationLineHeight = 1; // Fix for high-resolution device issues
switch (textDecorationLine) {
case TEXT_DECORATION_LINE_UNDERLINE:
this.ctx.fillRect(text.bounds.left, text.bounds.top + text.bounds.height - decorationLineHeight, text.bounds.width, decorationLineHeight);
break;
case TEXT_DECORATION_LINE_OVERLINE:
this.ctx.fillRect(text.bounds.left, text.bounds.top, text.bounds.width, decorationLineHeight);
break;
case TEXT_DECORATION_LINE_LINE_THROUGH:
this.ctx.fillRect(text.bounds.left, text.bounds.top + (text.bounds.height / 2) - (decorationLineHeight / 2), text.bounds.width, decorationLineHeight);
break;
}
});
}
...
}
Already PR |
Thank you for your suggestion. I have made the modifications accordingly. |
|
build failure: 'middle' is declared but its value is never read. |
Okay, I have made the changes. |
The original author is not active on github, can you push this on: https://github.com/yorickshan/html2canvas-pro, thx ❤️ (ps: 有什么问题也可以用中文交流 😄 ) |
好的 PR了 😄 |
Summary
This PR fixes/implements the following bugs/features
Closing issues
Fixes #2238 #1624 #2107