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

Map canvas sizing broken when contained in a transform scale container in v2.5.0 #11085

Closed
jrohland opened this issue Oct 5, 2021 · 11 comments · Fixed by #11310
Closed

Map canvas sizing broken when contained in a transform scale container in v2.5.0 #11085

jrohland opened this issue Oct 5, 2021 · 11 comments · Fixed by #11310
Assignees
Labels

Comments

@jrohland
Copy link

jrohland commented Oct 5, 2021

After updating to v2.5.0 we've noticed that maps we have contained inside of a container that has a transform scale css set are no longer updating their canvas height and width to fill the container size properly.

mapbox-gl-js version: 2.5.0

browser: Firefox

Steps to Trigger Behavior

  1. Place map inside of a div with a css property of transform(scale: 0.5);

Link to Demonstration

I've created a jsfiddle to showcase this issue: https://jsfiddle.net/jesserohland/hLkoys52/5/

This is a regression from v2.4.1 where this previously worked. https://jsfiddle.net/jesserohland/hLkoys52/6/

@SnailBones
Copy link
Contributor

Might be introduced by #10936

@SnailBones
Copy link
Contributor

SnailBones commented Oct 5, 2021

Confirmed that this is caused by #10936, this occurs after that PR was merged but not before.

@avpeery
Copy link
Contributor

avpeery commented Oct 5, 2021

@SnailBones .getBoundingClientRect() replaced clientWidth and clientHeight in favor of getting more precise calculations. However, .getBoundingClientRect() takes in CSS transforms before returning its values. Replacing getBoundingClientRect here to clientWidth and clientHeight or offsetWidth and offsetHeight would fix the transform issue.

@chrisgervang
Copy link

chrisgervang commented Oct 14, 2021

Thank you for reporting this issue and working on a fix. It's very convenient to be able to apply transform: scale() when building resizable UI panels and keep a fixed map viewport. We couldn't find a way to keep the the viewport consistent when resizing the map since they are internally set (prior to 2.5) by clientWidth/Height, so instead css scaling was the way we achieved this.

@felix-ht
Copy link

@avpeery any idea when this will be released? Feels like this should be cherry-picked into a 2.6.2 release (and 2.5.2)

@treehousetim
Copy link

@avpeery any idea when this will be released? Feels like this should be cherry-picked into a 2.6.2 release (and 2.5.2)

I also am wondering when we might be able to see this fixed.

Thank you.

@avpeery
Copy link
Contributor

avpeery commented Feb 3, 2022

@treehousetim @felix-ht @chrisgervang @jrohland

@avpeery any idea when this will be released? Feels like this should be cherry-picked into a 2.6.2 release (and 2.5.2)

The fix is included in the recent 2.7.0 release!

@jrohland
Copy link
Author

jrohland commented Feb 8, 2022

@avpeery I just tried out 2.7.0 and while it looks like the sizing is fixed when using a single container with a transform: scale() style, I'm still seeing sizing issues when scaling is used in conjunction with transform: translate(). We have a use case where we have an outer container which is scaled down and our map container is positioned via translate(). I have created a jsfiddle using 2.7.0 to showcase this bug.

https://jsfiddle.net/jesserohland/40kzeg7x/2/

This use case was previous working with 2.4.1 as visible here: https://jsfiddle.net/jesserohland/40kzeg7x/1/

Thanks for the initial fix!

@ryanhamley
Copy link
Contributor

@avpeery This appears to be caused by this line which prevents the map container from having more than one transformed parent element. It seems to work as expected if you make it while (el). Do you remember why this line checks for !transformValues?

@jrohland
Copy link
Author

jrohland commented Feb 9, 2022

@ryanhamley @avpeery Should I open up a new issue to track this since this original issue was resolved?

@ryanhamley
Copy link
Contributor

I opened a new bug ticket #11492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants