mapbox / mapbox-maps-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and Metal
https://www.mapbox.com/mapbox-mobile-sdk
Other
469 stars 152 forks source link

Incorrect scale bar width after resizing map view #413

Open datwelk opened 3 years ago

datwelk commented 3 years ago

Environment

Observed behavior and steps to reproduce

The right half of the scale bar is partly positioned off-screen.

image

Expected behavior

The scale bar is positioned within the bounds of the map view.

Notes / preliminary analysis

In the view hierarchy inspector, we can see that the first subview of MapboxScaleBarOrnamentView, the staticContainerView, is correctly positioned within the map's bounds. Its subview dynamicContainerView however, is positioned partially off-screen.

mapbox_scalebar

If the map is initialized with a larger fixed with, and it is resized again thereafter, the scale bar is positioned too far from the right edge:

image

In this case the static container view is larger than its dynamically calculated content:

image

Most likely this is caused by this line calculating the maximum width of the static container. The map was initialized with a width of 200 points, so 50% of that results in a max width of 100 points. The max width constraint is never updated when the scale bar view changes size. I would suggest that the line above adds a constraint relative to the superview's width, as opposed to a fixed constant contraint:

staticContainerView.widthAnchor.constraint(equalTo: self.widthAnchor, multiplier: 0.5).isActive = true

However this will not fix the issue: the dynamic content is laid out from left-to-right, so we will still see a gap on the right side between the scale bar and the map edge, depending on the map's actual zoom level. The dynamic content layout logic needs to change to lay out the bars from right-to-left instead of LTR if the ornament's position is on the right side of the map instead of the left.

Additional links and references

datwelk commented 3 years ago

The positioning part of this issue is fixed by https://github.com/mapbox/mapbox-maps-ios/pull/436. That PR does not fix the width issue.

ZiZasaurus commented 2 years ago

@datwelk I've tried your recommendation with a modification and, while it seems to be part of the solution, there are other issues that have emerged and we will need to do a deeper dive to redesign this class.

datwelk commented 2 years ago

Hi @ZiZasaurus and @tyofemi - what's the status of this ticket?

datwelk commented 2 years ago

Hi @tyofemi @ZiZasaurus what is the status of this issue?