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

Make style properties null-resettable; fix property function roundtripping #6168

Merged
merged 9 commits into from
Aug 27, 2016

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Aug 26, 2016

This PR includes several interwoven changes, all related to the runtime styling API:

/cc @frederoni

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Aug 26, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Aug 26, 2016
@1ec5 1ec5 self-assigned this Aug 26, 2016
@1ec5 1ec5 added the tests label Aug 26, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Aug 26, 2016

Every enumeration-typed property test is failing on the iOS build bot:

testSymbolLayer, (([(NSValue *)gLayer.iconTextFit isEqualToValue:[MGLRuntimeStylingHelper testEnum:MGLSymbolStyleLayerIconTextFitHeight type:@encode(MGLSymbolStyleLayerIconTextFit)]]) is true) failed

However, I’m unable to reproduce this failure locally.

If the property is unset, return the default value.

Fixes #6126.
@1ec5 1ec5 force-pushed the 1ec5-style-null-resettable-6126 branch from 5bd4ad8 to 34901b0 Compare August 26, 2016 22:40
@1ec5
Copy link
Contributor Author

1ec5 commented Aug 26, 2016

The test failures turned out to be legitimate. The tests caught yet another roundtripping issue.

case 'color':
let color = parseColor(value);
if (!color) {
throw new Error(`unrecognized color format in default value of ${property.name}`);
}
if (color.r === 0 && color.g === 0 && color.b === 0 && color.a === 0) {
return '`clearColor`';
return '`NSColor.clearColor` or `UIColor.clearColor`';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it seems to me that the iOS variant should come before macOS’s.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rest assured, I was being completely objective and dispassionate by alphabetizing the items in this list. 😉

Copy link
Contributor Author

@1ec5 1ec5 Aug 27, 2016

Choose a reason for hiding this comment

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

The entire declaration and its comment are now duplicated once per platform using conditional compilation macros. Each comment says only NSColor or UIColor as appropriate. Out of respect for iOS’s primacy, the UIKit version comes first.

@friedbunny
Copy link
Contributor

Other than my iOS partisanship, this LGTM. 👍

1ec5 added 8 commits August 26, 2016 22:27
The documentation for runtime style API properties now spells out the value of the default type, to help developers infer the type of the property itself.
Also support unsetting enumeration-typed properties.
Duplicate each color-typed property once per platform using conditional compilation macros. Let’s just hope we never have to throw CGColor into the mix.
@1ec5 1ec5 force-pushed the 1ec5-style-null-resettable-6126 branch from 34901b0 to f4be826 Compare August 27, 2016 05:28
@1ec5 1ec5 merged commit f4be826 into master Aug 27, 2016
@1ec5 1ec5 deleted the 1ec5-style-null-resettable-6126 branch August 27, 2016 21:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling tests
Projects
None yet
2 participants