[iOS] Enable developers to change position of ornaments #13911

lloydsheng · 2019-02-12T09:39:44Z

The PR was just a copy for #13556. I use the same branch lloyd-ui-position and fixed #13873.

Test on multiple size devices and systems

Device	❌ Navigation Bar	✅ Navigation Bar
iPhone XS Max - 12.1
iPhone X - 12.1
iPhone 6 Plus - 9.0
iPhone5 9.0

/cc @chriswu42 @chloekraw

julianrex

@captainbarbosa and I have gone over this PR - apart from the following performance concern this looks good.

Please look at removing the call to [self installScaleBarConstraints] in -updateScaleBar - this will probably cause a cascade of changes though - sorry!

platform/ios/CHANGELOG.md

platform/ios/app/MBXOrnamentsViewController.m

platform/ios/src/MGLMapView.h

julianrex · 2019-02-15T21:51:49Z

platform/ios/src/UIView+MGLAdditions.m

+    if (@available(iOS 11.0, *)) {
+        return self.safeAreaLayoutGuide.leadingAnchor;
+    } else {
+        return self.leadingAnchor;


Why doesn't this and mgl_safeTrailingAnchor also need to check automaticallyAdjustsScrollViewInsets?

If I was right the automaticallyAdjustsScrollViewInsets just affect the top anchor and the bottom anchor.

That I do not know. Can we double check?

platform/ios/test/MGLMapViewLayoutTests.m

platform/ios/CHANGELOG.md

fabian-guerra

So far this looks good, just minor adjustments.

I tested on iOS 9/11/12, my tests consisted on:

Zoom in/out.
Modify ornament's offset.
Rotating the map.
Time profiling.

platform/ios/CHANGELOG.md

platform/ios/src/MGLMapView.mm

julianrex · 2019-03-06T15:14:47Z

platform/ios/src/UIView+MGLAdditions.m

+    if (@available(iOS 11.0, *)) {
+        return self.safeAreaLayoutGuide.leadingAnchor;
+    } else {
+        return self.leadingAnchor;


That I do not know. Can we double check?

platform/ios/src/MGLMapView.h

julianrex · 2019-03-06T15:42:33Z

platform/ios/src/UIView+MGLAdditions.m

+            return viewController.topLayoutGuide.bottomAnchor;
+        }
+        else {
+            return self.topAnchor;


IIRC these anchors were introduced in iOS 9. What are your plans for iOS 8?

julianrex · 2019-03-06T15:44:56Z

platform/ios/src/MGLMapView.mm

-                                   constant:8.0 + self.contentInset.right]];
-    [containerView addConstraints:self.attributionButtonConstraints];
+- (void)setScaleBarOffset:(CGPoint)scaleBarOffset {
+    NSAssert(CGRectContainsPoint(self.bounds, scaleBarOffset), @"The position offset of the scale bar should within the mapview boundaries");


These asserts don't make sense since the offset is a delta, not a position.

As discussed, we should remove these asserts from this PR and address in a subsequent PR

platform/ios/src/MGLMapView.mm

friedbunny · 2019-03-07T23:47:58Z

platform/ios/src/MGLScaleBar.mm

@@ -185,7 +185,7 @@ - (void)commonInit {
 #pragma mark - Dimensions

 - (CGSize)intrinsicContentSize {
-    return CGSizeMake(self.actualWidth + self.lastLabelWidth/2, 16);
+    return CGSizeMake(ceil(self.actualWidth + self.lastLabelWidth/2), 16);


MGLPointRounded may be a better choice here than ceil, since it’s meant to account for pixel scale.

platform/ios/test/MGLMapViewLayoutTests.m

platform/ios/src/MGLMapView.mm

…tSize

julianrex

Thank you!

…of mapview ornaments (#13863) * Cherry-pick PR #13556 to adding options to customize positions of mapbox ornaments

lloydsheng requested review from a team, fabian-guerra and julianrex February 12, 2019 09:39

lloydsheng changed the title ~~[WIP] Enable developers to change position of ornaments~~ [iOS] Enable developers to change position of ornaments Feb 12, 2019

captainbarbosa self-requested a review February 13, 2019 21:06

captainbarbosa added the pair-review label Feb 13, 2019

julianrex suggested changes Feb 15, 2019

View reviewed changes

julianrex mentioned this pull request Feb 20, 2019

Allow scale bar to be positioned elsewhere similar to how MKMapView allows this #12729

Closed

captainbarbosa approved these changes Feb 26, 2019

View reviewed changes

lloydsheng requested a review from julianrex February 27, 2019 10:15

friedbunny added the iOS Mapbox Maps SDK for iOS label Feb 27, 2019

friedbunny added this to the release-l milestone Feb 27, 2019

friedbunny assigned lloydsheng Feb 27, 2019

friedbunny added the feature label Feb 27, 2019

friedbunny reviewed Feb 27, 2019

View reviewed changes

platform/ios/CHANGELOG.md Outdated Show resolved Hide resolved

julianrex mentioned this pull request Feb 28, 2019

MGLMapView contentInset top is ignored for compass positioning #11068

Closed

lloydsheng force-pushed the lloyd-ui-position branch from 60d9264 to 3c8dbac Compare March 1, 2019 09:25

fabian-guerra mentioned this pull request Mar 4, 2019

Adopt layout anchors #12147

Closed

2 tasks

fabian-guerra reviewed Mar 5, 2019

View reviewed changes

julianrex reviewed Mar 6, 2019

View reviewed changes

julianrex requested a review from friedbunny March 6, 2019 15:56

friedbunny reviewed Mar 7, 2019

View reviewed changes

friedbunny reviewed Mar 8, 2019

View reviewed changes

platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved

platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved

friedbunny reviewed Mar 8, 2019

View reviewed changes

platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved

This was referenced Mar 8, 2019

[ios] Replace ornament margin assert with NSException (follow on to #13911) #14065

Closed

[ios] Evaluate performance of removing/adding constraints in updateScaleBar #14066

Closed

Syamsul88 approved these changes Mar 8, 2019

View reviewed changes

lloydsheng added 3 commits March 11, 2019 17:49

APIs for change position of UI elements

d670105

Support UI elements size changing

930bae5

Move scalebar to top left

f98ddc5

lloydsheng added 17 commits March 11, 2019 17:49

Correct top and bottom constraint setting

359c7fa

add negative check

fe79b31

Improve log description

a183b18

Add ornaments manual test demo

78535f6

Add ornament tests

05f214d

Fix misspelled method name

274c61d

Correct format specifier

354cfbc

Fix retain cycle

0087289

Use anchors APIs; Remove iOS8 layout code

49c3df6

update sdk files

678f6b4

Fix scalebar position issue

2c389d4

Fix issues found in the review

b5fd997

Improve scalebar layout performance; Adopt scalebar's intrinsicConten…

e97f2d5

…tSize

Fix tests issue

f16bc4d

Update changelog

0c5a711

Improve offset check; Imporove changelog

4bf0245

Use proper API name; Fix code style issues

fda7d18

lloydsheng force-pushed the lloyd-ui-position branch from 0271a27 to fda7d18 Compare March 11, 2019 09:56

Remove tests

38c9016

lloydsheng requested review from julianrex and friedbunny March 11, 2019 15:00

julianrex approved these changes Mar 11, 2019

View reviewed changes

lloydsheng merged commit 60ceac5 into master Mar 11, 2019

lloydsheng deleted the lloyd-ui-position branch March 13, 2019 03:40

lloydsheng mentioned this pull request Mar 13, 2019

[ios] Cherry-pick PR #13911 to adding options to customize positions of mapview ornaments #13863

Merged

lloydsheng added a commit that referenced this pull request Mar 20, 2019

[ios] Cherry-pick PR #13911 to adding options to customize positions …

dab3ab7

…of mapview ornaments (#13863) * Cherry-pick PR #13556 to adding options to customize positions of mapbox ornaments

pappalar mentioned this pull request Apr 23, 2019

iOS ScaleBar is not movable via contentInset #14487

Closed

lloydsheng mentioned this pull request Apr 25, 2019

Add mapview ornaments layout example mapbox/ios-sdk-examples#297

Closed

pappalar mentioned this pull request Oct 30, 2019

Issues/Proposals on Ornament Margins: (Follow on #13911) mapbox/mapbox-gl-native-ios#42

Closed

1ec5 mentioned this pull request Nov 24, 2020

Allow the application to curate and customize floating map buttons mapbox/mapbox-navigation-ios#2749

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Enable developers to change position of ornaments #13911

[iOS] Enable developers to change position of ornaments #13911

lloydsheng commented Feb 12, 2019 •

edited

Loading

julianrex left a comment

julianrex Feb 15, 2019

lloydsheng Feb 27, 2019

julianrex Mar 6, 2019

fabian-guerra left a comment

julianrex Mar 6, 2019

julianrex Mar 6, 2019

julianrex Mar 6, 2019

julianrex Mar 8, 2019

friedbunny Mar 7, 2019

julianrex left a comment

[iOS] Enable developers to change position of ornaments #13911

[iOS] Enable developers to change position of ornaments #13911

Conversation

lloydsheng commented Feb 12, 2019 • edited Loading

Test on multiple size devices and systems

julianrex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianrex left a comment

Choose a reason for hiding this comment

lloydsheng commented Feb 12, 2019 •

edited

Loading