Conversation
1ec5
left a comment
1ec5
left a comment
There was a problem hiding this comment.
There seems to be too much whitespace at the bottom of the turn banner compared to the top (where the status bar takes up some of the space).
| <constraint firstItem="c3B-6D-Ukh" firstAttribute="leading" secondItem="dhM-Ng-gx5" secondAttribute="leading" id="Dcy-eQ-VCk"/> | ||
| <constraint firstAttribute="bottom" secondItem="gkj-hG-NxS" secondAttribute="bottom" constant="19" id="EBu-yb-gc6"/> | ||
| <constraint firstAttribute="bottom" secondItem="c3B-6D-Ukh" secondAttribute="bottom" id="Ofy-g6-EBl"/> | ||
| <constraint firstItem="gkj-hG-NxS" firstAttribute="leading" secondItem="c3B-6D-Ukh" secondAttribute="trailing" id="mWA-J9-gaq"/> |
There was a problem hiding this comment.
There needs to be a gap between the right edge of the street name label (and distance label) and the left edge of the shield view. The gap should be the same width as the right margin of the shield view and ideally the same width as the gap between the maneuver icon and the street name label.
There was a problem hiding this comment.
Added 8px spacing in the horizontal stack view.
|
Changed word wrap to truncate tail and 50% minimum font scale. Also added FBSnapshotTestCase and fixtures to verify single and multiline. We should add a few more cases to cover w/ and w/o a visible shield. |
frederoni
force-pushed
the
header-updarte
branch
3 times, most recently
from
64d5e9d to
ba9ba2d
Compare
|
@1ec5 I think this ready for review now |
|
Also noting, we were only showing lanes when paging through steps, added them back in 51f9e1a |
1ec5
left a comment
1ec5
left a comment
There was a problem hiding this comment.
testManeuverViewSingleLine@2x.png doesn’t show much of a gap between the street name label and the shield view – do you know why that might be?
MapboxNavigation/Abbreviations.swift
Outdated
|
|
||
| /// Returns the string abbreviated only as much as necessary to fit the given width and font. | ||
| func abbreviated(width: CGFloat, font: UIFont) -> String { | ||
| func abbreviated(bounds: CGRect, font: UIFont) -> String { |
There was a problem hiding this comment.
This method should be called abbreviated(toFit:font:), not abbreviated(bounds:font:). The relationship between the method name and the argument value should be clear when reading the method call.
| maneuverViewController.streetLabel.text = routeStepFormatter.string(for: step)?.abbreviated(bounds: streetLabelBounds, font: streetLabelFont) | ||
| } | ||
| maneuverViewController.distanceLabel.text = step!.distance > 0 ? distanceFormatter.string(from: step!.distance) : "" | ||
| maneuverViewController.distanceText = step!.distance > 0 ? distanceFormatter.string(from: step!.distance) : nil |
There was a problem hiding this comment.
It would be cleaner to replace the distanceText property with a distance property whose didSet takes care of formatting the numeric distance into a string.
I recorded the 2x before I added the spacing but I removed the unused reference images ( d6b8090 ) since we only test on iPhone 6s Plus 3x on Bitrise. Let's add more devices when we've verified that the snapshot tests are reliable and easy to implement in our workflow. |
3 participants
This changes the instruction header to:
To fix:
@frederoni could you take a look into the 2 line issues above?