Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios, macos] Add enablePlacementTransitions to MGLStyle. #13565

Merged
merged 2 commits into from
Dec 13, 2018
Merged

[ios, macos] Add enablePlacementTransitions to MGLStyle. #13565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f5e7d2
[ios, macos] Add enablePlacementTransitions to MGLStyle.
fabian-guerra Dec 12, 2018
fbba090
[ios, macos] Update changelogs.
fabian-guerra Dec 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions platform/darwin/src/MGLStyle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,13 @@ MGL_EXPORT
*/
@property (nonatomic) MGLTransition transition;

/**
A boolean value indicating whether label placement transitions are enabled.

The default value of this property is `YES`.
*/
@property (nonatomic, assign) BOOL enablePlacementTransitions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property has an imperative name, which makes it look like an action method in Objective-C:

// Initial state: placement transitions are disabled.
[style enablePlacementTransitions];
// Expected behavior: placement transitions are enabled.
// Actual behavior: nothing changed; return value discarded.

This property should be renamed to placementTransitionsEnabled or preferably performsPlacementTransitions. (enablesPlacementTransitions is problematic because it’s the developer, not the style, that enables them.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#13604


/**
Returns a source with the given identifier in the current style.

Expand Down
13 changes: 13 additions & 0 deletions platform/darwin/src/MGLStyle.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,19 @@ - (MGLTransition)transition
return MGLTransitionFromOptions(transitionOptions);
}

- (void)setEnablePlacementTransitions:(BOOL)enablePlacementTransitions
{
mbgl::style::TransitionOptions transitionOptions = self.rawStyle->getTransitionOptions();
transitionOptions.enablePlacementTransitions = static_cast<bool>(enablePlacementTransitions);
self.rawStyle->setTransitionOptions(transitionOptions);
}

- (BOOL)enablePlacementTransitions
{
mbgl::style::TransitionOptions transitionOptions = self.rawStyle->getTransitionOptions();
return transitionOptions.enablePlacementTransitions;
}

#pragma mark Style light

- (void)setLight:(MGLLight *)light
Expand Down
20 changes: 20 additions & 0 deletions platform/darwin/test/MGLStyleTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,24 @@ - (void)testLanguageMatching {
}
}

#pragma mark Transition tests

- (void)testTransition
{
MGLTransition transitionTest = MGLTransitionMake(5, 4);

self.style.transition = transitionTest;

XCTAssert(self.style.transition.delay == transitionTest.delay);
XCTAssert(self.style.transition.duration == transitionTest.duration);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also need to test enablePlacementTransitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I split the functionality in transition and enablePlacementTransitions I'm testing the latter in it's own test method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I'm thinking of a previous discussion I saw on the previous PR!

}

- (void)testEnablePlacementTransition
{
XCTAssertTrue(self.style.enablePlacementTransitions, @"The default value for enabling placement transitions should be YES.");

self.style.enablePlacementTransitions = NO;
XCTAssertFalse(self.style.enablePlacementTransitions, @"Enabling placement transitions should be NO.");
}

@end
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
* Added support for setting `MGLCollisionBehaviorPre4_0` in `NSUserDefaults`. ([#13426](https://github.com/mapbox/mapbox-gl-native/pull/13426))
* `-[MGLStyle localizeLabelsIntoLocale:]` and `-[NSExpression(MGLAdditions) mgl_expressionLocalizedIntoLocale:]` can automatically localize styles that use version 8 of the Mapbox Streets source. ([#13481](https://github.com/mapbox/mapbox-gl-native/pull/13481))
* Fixed symbol flickering during instantaneous transitions. ([#13535](https://github.com/mapbox/mapbox-gl-native/pull/13535))
* Added an `MGLStyle.enablePlacementTransitions` property to control how long it takes for collided labels to fade out. ([#13565](https://github.com/mapbox/mapbox-gl-native/pull/13565))

### Map snapshots

Expand Down
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Added support for setting `MGLCollisionBehaviorPre4_0` in `NSUserDefaults`. ([#13426](https://github.com/mapbox/mapbox-gl-native/pull/13426))
* Added support for automatic localization of version 8 of the Mapbox Streets source. ([#13481](https://github.com/mapbox/mapbox-gl-native/pull/13481))
* Fixed symbol flickering during instantaneous transitions. ([#13535](https://github.com/mapbox/mapbox-gl-native/pull/13535))
* Added an `MGLStyle.enablePlacementTransitions` property to control how long it takes for collided labels to fade out. ([#13565](https://github.com/mapbox/mapbox-gl-native/pull/13565))

### Other changes

Expand Down