Refactor offsetHeight use float-compat API #948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
2 times, most recently
from
aceb59c
to
5639ca1
Compare
c69-addepar
reviewed
May 3, 2022
c69-addepar
reviewed
May 3, 2022
c69-addepar
reviewed
May 3, 2022
c69-addepar
reviewed
May 3, 2022
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
2 times, most recently
from
79aa4f2
to
77d3950
Compare
c69-addepar
reviewed
May 6, 2022
| @@ -29,18 +40,52 @@ const Header = PageObject.extend({ | |||
|
|
|||
| /** | |||
| * Retrieves selected header cell width. | |||
| * | |||
| * Deprecated: offsetWidth returns a rounded integer, and so can | |||
There was a problem hiding this comment.
nit: use @deprecated jsdoc tag
c69-addepar
reviewed
May 6, 2022
| */ | ||
| get width() { | ||
| return findElement(this).offsetWidth; | ||
| }, | ||
|
|
||
| /** | ||
| * Retrieves selected header cell height. | ||
| * | ||
| * Deprecated: offsetHeight returns a rounded integer, and so can |
c69-addepar
reviewed
May 6, 2022
| /** | ||
| * Resizes this column by dragging right border several pixels. | ||
| /* | ||
| * This API is deprecated, prefer the more explicit resizeLogicalSize. |
c69-addepar
reviewed
May 6, 2022
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
from
77d3950
to
f3eb2b4
Compare
Closed
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
from
f3eb2b4
to
dfec26c
Compare
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
2 times, most recently
from
56abd37
to
675d8fb
Compare
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
9 times, most recently
from
f6e3a12
to
5e417a5
Compare
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
from
5e417a5
to
6ebd499
Compare
offsetHeight returns an integer, making it a poor API to base the determination of element scale on. Instead, use an alternative and precise API. Only allow a reasonable amount of precision by decimal place, and for cases where the scale number is reasonably rounded do so. The offsetHeight method includes padding in returned sizes, where getComputedStyle does not. Check them against each other, and if they diverge signifcantly (>1) throw an exception. To make ember-table itself compatible with this appraoch, pass the `scale` value around the system a bit more instead of trying to derive it in many places.
mixonic
force-pushed
the
mixonic/offsetHeight-is-int
branch
from
6ebd499
to
3aed24a
Compare
mmun
approved these changes
Aug 23, 2023
|
I've confirmed in an internal codebase that this improves how well a table can match the intended container it is in. Landing it, and I'll but a beta/preview release for Ember Table v6 sometime soon. |
|
Released in v6.0.0-0 prerelease |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
offsetHeight, per MDN, is an API which returns integer values. This makes is inappropriate to use in calculations of current UI scale (usually from a transform) as it rounds any floating point height into an integer.This PR changes the
getScalefunction to usegetComputedStyleto read off the logical height of the element. This API supports greater precision (to at least 3 digits) though it also may not provide the same level of precision asgetBoundingClientRect.This prevents the positioning data for cells (which is impacted by
scale) from being corrupted by heights which are at decimal precision.