Require iOS 10.0 #2206
Merged
Require iOS 10.0 #2206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1ec5
added
build
|
The CocoaPods installation test was failing because of a warning about UILocalNotification being deprecated on iOS 10.0 and above. 09c376abed41744b1ed996ea762c9c4ff47273c6 migrates to the UserNotifications framework, removing a hack we put in for replacing any already present notification. I also took the opportunity to revamp the notifications. Instead of displaying the plain-text representation of the spoken instruction, which can easily get truncated, the notification now mirrors the turn banner more closely, displaying the primary instruction text, secondary instruction text, and maneuver icon.
|
frederoni
approved these changes
Aug 19, 2019
.circleci/config.yml
Outdated
| xcode: "10.1.0" | ||
| iOS: "9.3" | ||
| iOS: "10.3.1" | ||
| delete_private_deps: true |
There was a problem hiding this comment.
nit: this flag is no longer necessary. We removed private deps because some or one of them didn't support iOS 9. Technically, we can start testing on iOS 10 but we would have to generate snapshot fixtures.
|
Could we add a iOS 13.0 CI bot as part of this effort? |
|
Spurred in part by the Swift 5 warnings in #2232 (comment), I rewrote DistanceFormatter to inherit from Formatter and wrap MeasurementFormatter instead of inheriting from LengthFormatter. In the process, I removed some cruft stemming from the old vulgar fraction functionality removed in #383. These changes needed to happen in this PR because Measurement and MeasurementFormatter were introduced in iOS 10. |
frederoni
reviewed
Sep 18, 2019
| let value = distanceFormatter.measurement(of: distance).value | ||
| XCTAssertEqual(distanceFormatter.numberFormatter.string(from: value as NSNumber), quantity) | ||
| } | ||
| let value = Measurement(distance: distance).localized(into: distanceFormatter.locale).value |
1ec5
commented
Sep 18, 2019
| assertDistance(2_500, displayed: "2.5 km", quantity: "2.5") | ||
| assertDistance(2_900, displayed: "2.9 km", quantity: "2.9") | ||
| assertDistance(2_500, displayed: "2,5 km", quantity: "2,5") | ||
| assertDistance(2_900, displayed: "2,9 km", quantity: "2,9") |
There was a problem hiding this comment.
I don’t think this indicates a bug in master, just a shortcoming of how the test was previously written. The test previously set the locale of the number formatter above, but the locale would be changed back to nationalizedCurrent within the formatter. Now the locale is set on the distance formatter, which takes care of the measurement formatter’s locale.
|
The Xcode_10.2_iOS_12.2_CP_update bot keeps failing with this compiler error: The module does have access to the public |
JThramer
approved these changes
Sep 25, 2019
|
I wonder if there’s a way to override the MapboxCoreNavigation commit hash that CocoaPods uses when linting the MapboxNavigation podspec. |
|
a6e1a9e modifies the CI configuration to no longer lint MapboxNavigation.podspec. MapboxNavigation fails the linter between releases because it depends on MapboxCoreNavigation. The linter installs the latest release of MapboxCoreNavigation, which potentially has a different set of public symbols than what’s in the current branch. Moreover, at release time, it fails the linter because the new release of MapboxCoreNavigation hasn’t been published yet. |
1ec5
added
op-ex
|
iOS 13 seems to have changed some default behaviors around number formatting. Specifically, rounding 0.5 now seems to follow the even-odd rule rather than rounding away from zero. There are also numerous errors related to the Hindi localization switching from Indian numerals to European numerals: |
Actually, the selected threshold for this test is one that has a The difference between iOS 12 and iOS 13 appears to be caused by a different conversion factor between meters and miles: 1609.34 meters per mile on iOS 12 versus 1609.344 on iOS 13. |
In iOS 13, the Hindi localization defaults to Latin-script numbers but offers an option to switch back to Devanagari-script numerals:
The BCP 47 code for Hindi with Devanagari numbers would be |
|
Still need to regenerate snapshot test fixtures. Also, this simulation test is failing:
|
The minimum deployment target is now iOS 10.0. Updated the CircleCI configuration to test on iOS 10.3.1 instead of 9.3.
Lock screen notifications are presented more reliably and more closely resemble instruction banners. Factored out a method that produces a UIImage representing a visual instruction’s maneuver.
Removed iOS 10 availability guards. Replaced LengthFormatter.Unit usage with Measurement. For now, a MeasurementFormatter is embedded inside DistanceFormatter.
DistanceFormatter no longer inherits from LengthFormatter. The quantity attribute now reflects any rounding that took place in the attributed string. Added strongly typed attributed string formatting methods. Extended Measurement with an initializer from CLLocationDistance and method for localizing to a locale-appropriate unit.
MapboxNavigation fails the linter between releases because it depends on MapboxCoreNavigation. The linter installs the latest release of MapboxCoreNavigation, which potentially has a different set of public symbols than what’s in the current branch. Moreover, at release time, it fails the linter because the new release of MapboxCoreNavigation hasn’t been published yet.
Different versions of iOS have different meter-mile conversion factors, so encapsulate conversions in Measurement.
iOS 13 uses a meter-mile conversion factor that happens to expose a floating-point precision error when the input value is halfway between two integers. This change applies a small rounding increment to avoid the imprecision.
The CPWindow doesn’t end up in the view hierarchy by itself on iOS 13, so the navigation service doesn’t start. This test is mainly about simulation mode, not about whether the navigation service starts automatically, so just start it automatically as if the view had loaded.
The simulated location source is populated when the navigation service begins, so the test failure indicates that the navigation service never began. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This library now requires a minimum deployment target of iOS 10.0 or above. iOS 9.x is no longer supported. This change is forced by mapbox/mapbox-directions-swift#379 and needed for continued feature development, including #2114.
Along the way:
Details on these changes below.
/cc @mapbox/navigation-ios @mapbox/driver-app