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

fix #6379: beef up runtime styling manual tests #6433

Merged
merged 1 commit into from
Sep 23, 2016

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Sep 22, 2016

Work in progress to test filtered fills & lines.

@incanus incanus added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Android Mapbox Maps SDK for Android tests ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold runtime styling labels Sep 22, 2016
@incanus incanus added this to the ios-v3.4.0 milestone Sep 22, 2016
@incanus incanus self-assigned this Sep 22, 2016
@mention-bot
Copy link

@incanus, thanks for your PR! By analyzing this pull request, we identified @1ec5, @friedbunny and @boundsj to be potential reviewers.

@incanus
Copy link
Contributor Author

incanus commented Sep 22, 2016

Got some examples bolted in for styling fills & lines using filters. Both feature Studio-created layers that are well under the normal map layers, showing off some of our runtime styling flexibility.

Map with states fill layer: fill in Texas

simulator screen shot sep 22 2016 12 33 01 pm

Map with counties line layer: restyle all Washington counties

simulator screen shot sep 22 2016 12 32 32 pm

Other than some minor flickering upon styling, things look good.

Next up is adding a numeric-type filter test to flex #6040 and the Android side.

[self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(31, -100) zoomLevel:3 animated:NO];

// after slight delay, fill in Texas
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^
Copy link
Contributor

@1ec5 1ec5 Sep 22, 2016

Choose a reason for hiding this comment

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

A hard-coded timeout would be unfortunate. We don't have a completion block–based way to set the style; in the meantime, you can do this in -mapViewDidFinishLoading: or the forthcoming -mapView:didFinishLoadingStyle: (#6412) perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It splits functionality up fairly crappily, but I see your point.

Copy link
Contributor

Choose a reason for hiding this comment

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

#6386 (comment) would mitigate this pain considerably, but it’s aways off.

Copy link
Contributor Author

@incanus incanus Sep 23, 2016

Choose a reason for hiding this comment

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

Looked into this with a patch such as the following:

https://gist.github.com/1ab15bce31631f5ba032a731bd0bed38

However, it happens too quickly, essentially immediately after the style finishing tiling in, causing continual flashing between that happening and then the filter style changes. I want to eliminate the possibility that anything is done as part of the initial style fill, hence the delay so that the user can clearly see things in one state, then the other, to confirm that the filtered styling is applied without any external tile refreshing happening.

Copy link
Contributor

@1ec5 1ec5 Sep 23, 2016

Choose a reason for hiding this comment

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

OK, in that case, can you document the reasoning here? Now that we’re getting the style from a local resource, I’m not as concerned about the time it’ll take to load it, but someone using iosapp as a learning resource shouldn’t be misled into thinking this is how you use the runtime styling API normally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

89bfeaa88eec7840ecee64c15f22e34d7f6bcbdf

- (void)styleFilteredFill
{
// set style and focus on Texas
[self.mapView setStyleURL:[NSURL URLWithString:@"mapbox://styles/justin/citc7ya7w00002jpa0z16cjah"]];
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only work with your access token, not the access token that the user enters. Instead, check in the style JSON as part of this project and refer to the bundled JSON file with a file URL or relative path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

63cdd45580602e9a7206dd6d39a76fd2a40887e4

MGLFillStyleLayer *regionsLayer = (MGLFillStyleLayer *)[self.mapView.style layerWithIdentifier:@"regions"];

// filter
regionsLayer.predicate = [NSCompoundPredicate andPredicateWithSubpredicates:@[
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be expressed as a single predicate format string using the AND operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

c3c9ca40aece3b08b9e63c2a13030e469452a845

@boundsj
Copy link
Contributor

boundsj commented Sep 22, 2016

Thanks for doing this. While you are at it, I think it'd be great to add cases for the soon to land #6316 to exercise tile template URLs and also something to cover styling 3rd party vector (and raster) source layers. Support for the later was added in the fix for #6251. A single example can cover this (see #6316 (comment))

@incanus
Copy link
Contributor Author

incanus commented Sep 23, 2016

Got this pretty well set on the Android side too.

@boundsj On the ones you mention, is it more useful to visually test the functionality or to just confirm that something happens? i.e. I'm wondering how much we can push that to automated tests. The things I'm doing here are specifically things that won't ever throw or error, but might not look right if a bug crept in.

@incanus
Copy link
Contributor Author

incanus commented Sep 23, 2016

screenshot_20160922-171050
screenshot_20160922-171112
screenshot_20160922-171232

@incanus incanus removed ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold macOS Mapbox Maps SDK for macOS labels Sep 23, 2016
@boundsj
Copy link
Contributor

boundsj commented Sep 23, 2016

@incanus might not look right if a bug crept in

I think that definition covers a lot of runtime styling cases including what I suggested. For example the MGLTileSet class that will be introduced in #6316 has its own set of options that affect how the map looks. Feel free to ignore my suggestion though if you don't see value in adding that case at this time.

@incanus incanus force-pushed the ios-android-runtime-styling-beefy branch from b15ca3e to 6daa62e Compare September 23, 2016 00:47
@incanus
Copy link
Contributor Author

incanus commented Sep 23, 2016

Squashed and force pushed. This 🚢 will be ready to ⛵ once tests pass.

@incanus incanus force-pushed the ios-android-runtime-styling-beefy branch from 6daa62e to 89bfeaa Compare September 23, 2016 00:51
@incanus
Copy link
Contributor Author

incanus commented Sep 23, 2016

Ugh, fixing Android local style JSON also first.

@incanus incanus force-pushed the ios-android-runtime-styling-beefy branch from 89bfeaa to b846f54 Compare September 23, 2016 01:14
@incanus incanus merged commit 6aa00af into master Sep 23, 2016
@incanus incanus deleted the ios-android-runtime-styling-beefy branch September 23, 2016 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android iOS Mapbox Maps SDK for iOS runtime styling tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants