Fix canvas sizing with cases that use the css property transform #11310
Conversation
There was a problem hiding this comment.
Great work on this @avpeery!
Would it be possible to add one more unit test to check that the transform(scale) regression doesn't happen again?
|
@SnailBones any chance to get this merged and released? |
|
@arindam1993 if you can give a review, that would be helpful too! is there potentially any case containerDimension() will get called when there is no container? If so, I can fix the removal of setting width and height to 0 before checking if there's a container. @SnailBones thanks for the review! |
src/ui/map.js
Outdated
| * @returns {object} | ||
| */ | ||
| _getContainerDimensions(): {width: number, height: number} { | ||
| return {width: this._containerWidth, height: this._containerHeight}; |
There was a problem hiding this comment.
Nit: we could use map._containerWidth and map. _containerHeight directly instead of adding one more method that wraps them to simplify the code — we always unwrap the values when using them anyway.
|
Can anyone comment on the release cadence and when this particular fix might get released? |
Issue #11085 demonstrates a regression introduced in v2.5.0. PR #10936 introduced using getBoundingClientRect to obtain the canvas width and height for more precise values; instead of using clientWidth and clientHeight. However, an edge case was discovered that getBoundingClientRect returns its values after CSS transformations.
In order to fix this issue, a check is done to see if getBoundingClientRect when rounded is not equal to clientWidth or clientHeight (this case would be indicative that a transform(scale) was used). The difference between getBoundingClientRect and clientWidth/clientHeight is calculated and applied to getBoundingClientRect to get the actual expected value.
Added properties _containerWidth and _containerHeight to map to eliminate need to reevaluate the transform difference in other areas where getBoundingClientRect is used to find container height and width.
The debug page was adapted to include transform, please manually test this page which is also a release testing page.
Before
After
Launch Checklist
mapbox-gl-jschangelog:<changelog>Canvas size evaluates to expected value when applying the css transform property</changelog>