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

Bugfix print area decimal #41

Closed
wants to merge 5 commits into from

Conversation

wlfrdssn
Copy link
Contributor

My fork is messed up.

Fit to page problem, showing red dashed line at certain scales.

Fit to page problem, showing red dashed line at certain scales.
Copy link

vercel bot commented Jan 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
o-scout ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2025 7:55pm

@perliedman
Copy link
Owner

perliedman commented Feb 8, 2025

Thanks, I did notice this issue while looking at your other PR.

I think the fix here is slightly wrong although it will most certainly work in most cases: toFixed will create a string, so we're really comparing these two numbers in alphabetical order. That will work as long as they have the same number of digits. For example, using this logic, this expression will return true: (99.1).toFixed(5)>=(1000.1).toFixed(5)

What we do want to check is that the print area isn't significantly larger than the page size, say 1e-5. I think this should work:

      // Check that print area isn't significantly larger
      // (1e-5 is the tolerance so we don't complain about tiny rounding errors)
      const isValid =
        pageSizeMm[0] + 1e-5 >= printAreaSizeMm[0] &&
        pageSizeMm[1] + 1e-5 >= printAreaSizeMm[1];

@wlfrdssn
Copy link
Contributor Author

wlfrdssn commented Feb 9, 2025

Oh! Didn't know it worked that way. Easy and clean solution you had on this. Why didn't I think of that :)

@perliedman perliedman closed this in 4054bcb Feb 9, 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.

2 participants