Retrieve additional map attributes to display reliable current road name. #1576

vincethecoder · 2018-07-24T21:53:40Z

Uses the shield and reflen attributes to get the shield image from the map style and inserts it into the current road name label as part of an attributed string. Includes a data model for the shields with the associated textColor based on factors such as Locale, RoadType and/or RoadClass.

(FYI: The style images provided already look crisp at the size needed for the navigationView.wayNameView's label).

TODO - Screenshots:

Addresses #1440

…tails for textColor and fillColor for current road feature.

… updated.

…ruct.

… Updated route map view controller to integrate the new changes in the highway shield struct.

… Updated route map view controller to integrate the new changes in the highway shield struct. Co-authored-by: Jerrad Thramer <[email protected]> Co-authored-by: Vincent Sam <[email protected]>

…om:mapbox/mapbox-navigation-ios into navigation/reliable_current_roadname_label

…lass used within the highway shield struct.

…ighway shield.

bsudekum

Looking @vincethecoder. I'm having a hard time getting a shield to show up, where can I test this? When simulating freeway drive in the sim, it still shows Junipero Serra Fwy.

bsudekum · 2018-07-27T16:11:37Z

MapboxNavigation/RouteMapViewController.swift

@@ -808,18 +809,56 @@ extension RouteMapViewController: NavigationViewDelegate {
                    } else {
                        currentName = nil
                    }
+
+                    if let line = feature as? MGLPolylineFeature {


We've already pulled this value out above. See if you can combine these if let statements.

bsudekum · 2018-07-27T16:21:24Z

MapboxNavigation/RouteMapViewController.swift

+    private func attributedString(withFont font: UIFont, shieldImage: UIImage, text: String, color: UIColor?) -> NSAttributedString {
+        let attachment = RoadNameLabelAttachment()
+        attachment.font = font
+        attachment.text = text


What are we getting out of RoadNameLabelAttachment over NSAttributedString directly? Having a new text prop sounds a little funny when there is already an initializer for string.

RoadNameLabelAttachment should handle the text and image composition. Our primary goal was to use the text prop as the accessibility text/hint for that road name label attachment image. However, it has been revised to allow the RoadNameLabelAttachment to handle the task of inserting the text in the image.

…f text and image for road name label attachment. Refactor to retrieve the appropriate attachment for current shields on a road along the route.

bsudekum · 2018-07-30T16:43:57Z

The text should be .white here I think.

vincethecoder · 2018-07-30T17:21:58Z

@bsudekum yes, that's correct! The textColor should be .white as indicated here. The captured image uploaded in the summary was the wrong file. It's been updated ✅

…the current polyline and multipolyline features. Updates to return the original full format of the rawValue init with HighwayShield.

vincethecoder · 2018-07-30T21:03:08Z

MapboxNavigation/RouteMapViewController.swift

+                    if let line = feature as? MGLPolylineFeature {
+                        if let name = line.attribute(forKey: "name") as? String {
+                            currentName = name
+                        } else if let line = feature as? MGLMultiPolylineFeature, let name = line.attribute(forKey: "name") as? String {


This line smells suspicious to me. It's existing logic that checks for line = feature as? MGLPolylineFeature and line = feature as? MGLMultiPolylineFeature within its scope. Needs further investigation why this exists.
/cc @bsudekum @1ec5

…e given road segment.

JThramer

Excited to get this into the wild! 🦁

bsudekum · 2018-07-31T20:15:27Z

I'm slightly concerned with design here. I think it's difficult to quickly understand what the little bubble with a shield means. It'd be interesting to see what this would look like where we spell out the ref as well to make this more understandable.

It'd look something like 🛡us-101.

1ec5 · 2018-07-31T20:18:53Z

🛡US 101 would look pretty redundant and not easy to come up with reliably, as we discovered in the original PR. However, I think something like 🛡Bayshore Highway would be reasonable (with the street name), even if normally we avoid freeway names in the UI. At least it would make the label look more consistent with a banner instruction.

bsudekum · 2018-07-31T20:21:38Z

@1ec5 the problem with that is the ol' ceremonial name problem. Junipero Serra Fwy is not helpful.

vincethecoder · 2018-08-01T16:36:54Z

@bsudekum, @1ec5 we should probably bring a designer to this conversation too.

/cc @taraniduncan @cjballard

bsudekum · 2018-08-01T16:41:44Z

To build on the prior comment, I have the same feeling about the single shield in the way name label as I would about a top banner that only has a single shield in it. IMO, when the top banner has only one element, it almost feels like a bug as it's not descriptive enough for the driver to fully understand what we're trying to tell them.

@vincethecoder do you want to give @1ec5's suggestion a try? It'd look something like

if name and ref
  display name and ref
else if name and !ref
  display name
else if !name and ref
  display ref
else 
  hide

vincethecoder · 2018-08-01T17:35:35Z

@bsudekum sure thing. Can do

…displayed.

vincethecoder · 2018-08-01T21:21:55Z

/cc @bsudekum @1ec5 @cjballard

JThramer · 2018-08-01T21:23:34Z

Wow, looks great @vincethecoder!

bsudekum · 2018-08-01T21:26:42Z

Yeah, I think this is looking much better @vincethecoder.

Cjballard-zz · 2018-08-01T21:50:36Z

Looks good to me @vincethecoder 👌

vincethecoder added 8 commits July 24, 2018 14:53

[GH-1440] - WIP Created a Highway shield struct to contain all the de…

fdd18cd

…tails for textColor and fillColor for current road feature.

[GH-1440] - Added TextPresenter which permits images to contain texts.

48f8e59

[GH-1440] - Modify styles to contain an attributed text.

cb4976e

[GH-1440] - Modify styles to contain an attributed text.

7874ada

[GH-1440] - Added function to insert a text in the current image.

6de1d55

[GH-1440] - Refactor to clean up certain function signature. Cartfile…

187fb0f

… updated.

[GH-1440] - Deleted un-used fill color added to the highway shield st…

776eda8

…ruct.

[GH-1440] - WIP - Struct to handle the parsing of highway shield icons.

4a0938a

vincethecoder self-assigned this Jul 24, 2018

vincethecoder added the wip label Jul 24, 2018

[GH-1440] - Modified the highway shield with concise enum variations.…

0907c18

… Updated route map view controller to integrate the new changes in the highway shield struct.

vincethecoder changed the title ~~[WIP] - Reliability of current road name label~~ Reliability of current road name label Jul 26, 2018

vincethecoder added the ✓ ready for review label Jul 26, 2018

vincethecoder and others added 5 commits July 26, 2018 11:31

[GH-1440] - Modified the highway shield with concise enum variations.…

ab7cd59

… Updated route map view controller to integrate the new changes in the highway shield struct. Co-authored-by: Jerrad Thramer <[email protected]> Co-authored-by: Vincent Sam <[email protected]>

Merge branch 'navigation/reliable_current_roadname_label' of github.c…

254b832

…om:mapbox/mapbox-navigation-ios into navigation/reliable_current_roadname_label

[GH-1440] - Refactored the closures for locale and locale with road c…

2494cc6

…lass used within the highway shield struct.

[GH-1440] - Renamed the insert method in the uiimage extension.

afc19bf

[GH-1440] - Bolden the text of the system font centered text in the h…

5c15b03

…ighway shield.

vincethecoder changed the title ~~Reliability of current road name label~~ Retrieve additional map attributes to display reliable current road name. Jul 26, 2018

[GH-1440] - Added missing enum types.

a5d75bf

bsudekum reviewed Jul 27, 2018

View reviewed changes

vincethecoder removed the wip label Jul 27, 2018

[GH-1440] - Added convenience initializer to handle the composition o…

795f553

…f text and image for road name label attachment. Refactor to retrieve the appropriate attachment for current shields on a road along the route.

vincethecoder added the wip label Jul 30, 2018

akitchen requested a review from 1ec5 July 30, 2018 18:26

[GH-1440] - Refactored the logic that retrieves the shield name from …

a76fafd

…the current polyline and multipolyline features. Updates to return the original full format of the rawValue init with HighwayShield.

vincethecoder commented Jul 30, 2018

View reviewed changes

[GH-1440] - Minor clean up.

32942de

vincethecoder requested a review from bsudekum July 31, 2018 15:58

vincethecoder added 3 commits July 31, 2018 13:20

[GH-1440] - Refactor to retrieve the data for shield and names for th…

0926a42

…e given road segment.

[GH-1440] - Minor clean up - removed extra spaces.

d33a3b9

[GH-1440] - Minor clean up - refactored variable names.

12f9954

vincethecoder removed the wip label Jul 31, 2018

Merge branch 'master' into navigation/reliable_current_roadname_label

5deaa9f

JThramer approved these changes Jul 31, 2018

View reviewed changes

[GH-1440] - Updates to include the road name when the road shield is …

aedb833

…displayed.

[GH-1440] - Minor refactor, to keep things aligned.

efbbb5a

bsudekum approved these changes Aug 2, 2018

View reviewed changes

vincethecoder merged commit f185b56 into master Aug 2, 2018

This was referenced Aug 18, 2018

Improve way name runtime styling performance mapbox/mapbox-navigation-android#1222

Closed

Support multi-icon symbols mapbox/mapbox-gl-js#4366

Open

Nine-part resizable images mapbox/mapbox-gl-js#4143

Closed

akitchen mentioned this pull request Oct 28, 2018

Road name / way name label should only be updated once per maneuver #1802

Closed

1ec5 deleted the navigation/reliable_current_roadname_label branch December 5, 2019 18:22

1ec5 mentioned this pull request Apr 8, 2020

Custom map style makes current road name label too tall, route shield too big #2355

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retrieve additional map attributes to display reliable current road name. #1576

Retrieve additional map attributes to display reliable current road name. #1576

vincethecoder commented Jul 24, 2018 •

edited

Loading

bsudekum left a comment

bsudekum Jul 27, 2018

bsudekum Jul 27, 2018

vincethecoder Jul 30, 2018

bsudekum commented Jul 30, 2018

vincethecoder commented Jul 30, 2018 •

edited

Loading

vincethecoder Jul 30, 2018

JThramer left a comment

bsudekum commented Jul 31, 2018

1ec5 commented Jul 31, 2018

bsudekum commented Jul 31, 2018

vincethecoder commented Aug 1, 2018

bsudekum commented Aug 1, 2018

vincethecoder commented Aug 1, 2018 •

edited

Loading

vincethecoder commented Aug 1, 2018

JThramer commented Aug 1, 2018

bsudekum commented Aug 1, 2018

Cjballard-zz commented Aug 1, 2018

Retrieve additional map attributes to display reliable current road name. #1576

Retrieve additional map attributes to display reliable current road name. #1576

Conversation

vincethecoder commented Jul 24, 2018 • edited Loading

TODO - Screenshots:

bsudekum left a comment

Choose a reason for hiding this comment

bsudekum Jul 27, 2018

Choose a reason for hiding this comment

bsudekum Jul 27, 2018

Choose a reason for hiding this comment

vincethecoder Jul 30, 2018

Choose a reason for hiding this comment

bsudekum commented Jul 30, 2018

vincethecoder commented Jul 30, 2018 • edited Loading

vincethecoder Jul 30, 2018

Choose a reason for hiding this comment

JThramer left a comment

Choose a reason for hiding this comment

bsudekum commented Jul 31, 2018

1ec5 commented Jul 31, 2018

bsudekum commented Jul 31, 2018

vincethecoder commented Aug 1, 2018

bsudekum commented Aug 1, 2018

vincethecoder commented Aug 1, 2018 • edited Loading

vincethecoder commented Aug 1, 2018

JThramer commented Aug 1, 2018

bsudekum commented Aug 1, 2018

Cjballard-zz commented Aug 1, 2018

vincethecoder commented Jul 24, 2018 •

edited

Loading

vincethecoder commented Jul 30, 2018 •

edited

Loading

vincethecoder commented Aug 1, 2018 •

edited

Loading