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

Tables where cell padding has been set to 0 lead to cropped/non-matching image #95

Closed
melink14 opened this issue Dec 30, 2022 · 8 comments · Fixed by #108
Closed

Tables where cell padding has been set to 0 lead to cropped/non-matching image #95

melink14 opened this issue Dec 30, 2022 · 8 comments · Fixed by #108

Comments

@melink14
Copy link

Use case: description, code

jsfiddle

In production, I have a large table and noticed that the bottom and left edges were cut off when saving to Png. Using fiddle I isolated the problem to setting the padding on table cells to 0.

In the fiddle the bottom edge is cut off and also the image is wider than the dom table so I guess there may be various small differences when padding is 0 in this case.

Seems related to #77 where the user also fixed the issue by adding padding. Tables naturally have 1px cell padding so this might not have been noticed in the general case.

image

Expected behavior

Image should be full table.

Actual behavior (stack traces, console logs etc)

Image doesn't match. Edges are cropped etc.

Library version

2.13.0

Browsers

  • Chrome 108
melink14 added a commit to melink14/BetrayalCheatSheet that referenced this issue Dec 30, 2022
In order to work around a bug in image rendering library, we can
no longer set cell padding to 0. The visual difference is small so the
tradeoff seems worth it.

See 1904labs/dom-to-image-more#95
IDisposable added a commit that referenced this issue Jan 12, 2023
Changed `getDefaultStyle` to create the element in the sandbox so it is pure (not tainted by CSS / ShadowDOM in the main document).
Never clone SCRIPT elements (that's just asking for trouble).
Removed `imagediff` package as it's not useful anymore.
@IDisposable
Copy link
Member

@melink14 You can set the padding to 0.000001px and it will work as expected :) It's a browser bug that I need to figure out how to report.

@IDisposable
Copy link
Member

Also, there will be a new release in the v2 chain tomorrow... just finalizing a bit of cleanup.

@IDisposable
Copy link
Member

See https://jsfiddle.net/4n52yzcg/119 for what that looks like (preview)

@zm-cttae
Copy link

@zm-cttae
Copy link

zm-cttae commented Jan 13, 2023

domtoimage.toSvg(
  document.getElementById('mywrap'),
  { onclone: node => node.querySelectorAll('th, td').forEach(el => el.style.padding = '0px') }
)
  .then(function(dataUrl) {
    var img = new Image();
    document.getElementById('result').appendChild(img);
    img.src = dataUrl;
  })
  .catch(function(e) {
    console.log(e);
  })

@melink14
Copy link
Author

@melink14 You can set the padding to 0.000001px and it will work as expected :) It's a browser bug that I need to figure out how to report.

Thanks for the work around!

IDisposable added a commit that referenced this issue Jan 15, 2023
* Work on table padding issue #95

Changed `getDefaultStyle` to create the element in the sandbox so it is pure (not tainted by CSS / ShadowDOM in the main document).
Never clone SCRIPT elements (that's just asking for trouble).
Removed `imagediff` package as it's not useful anymore.

* Add ability to insert the cloned node into the test

The allows inspecting the properties of the clone.

* Simplified the SVG

Removed the setting of the `top` and `left`
Set the `width` and `height` on the `foreignObject` element to the target size.
Removed setting of `width` and `height` on the `svg` element.

* Copy the size from the original object when possible

Falls back to the old `scrollWidth`/`scrollHeight` logic if the `width` and `height` aren't set in `px`.

* Revert to have `width` and `height` dimensions on SVG object.
IDisposable added a commit that referenced this issue Jan 15, 2023
@IDisposable
Copy link
Member

IDisposable commented Jan 16, 2023

Updated the jsfiddle https://jsfiddle.net/h859jpcf/ to show the work-around, and also demo how use the onclone event to grab exactly what was cloned to pass to the SVG generator.

@zm-cttae
Copy link

zm-cttae commented Feb 4, 2023

It will be another 3-6 months for Chromium bug #1406929 to land in peoples' devices (version target 111).

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 a pull request may close this issue.

3 participants