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 buggy header shadow #2494

Conversation

PegasisForever
Copy link

Fix issue: #2488

Explained in this comment: #2488 (comment)

@squidfunk
Copy link
Owner

squidfunk commented Mar 26, 2021

Thanks for investigating. So, in the end, it's only a rounding error, right? Does your solution also work with tabs and the announcement bar?

@PegasisForever
Copy link
Author

Ya it's just a rounding error, my code works with tabs and announcement bar, tested on the official docs site.

@squidfunk
Copy link
Owner

Thanks for the PR and your investigation. It's definitely related to rounding errors. However, the fix doesn't solve the issue in its entirety. When I zoom out, the header shadow is still not visible with your fix applied. The problem stems from the fact that getElementSize uses the offsetWidth and offsetHeight properties, while watchElementSize returns DOMRect objects. The former is the effective size as rendered (thus, no floating point values), the latter what has been computed during layout calculation (floating point values).

We definitely need to make those computations consistent, but we can't just use Math.round, because different browsers round differently, so the behavior would not be consistent.

@squidfunk
Copy link
Owner

Superseded by f43ef69 – I fixed the root cause.

@squidfunk squidfunk closed this Mar 28, 2021
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.

None yet

2 participants