Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update style color palette for v3 #1366

Merged
merged 17 commits into from May 9, 2018
Merged

Update style color palette for v3 #1366

merged 17 commits into from May 9, 2018

Conversation

frederoni
Copy link
Contributor

@frederoni frederoni commented May 2, 2018

Partially closes #1347

This PR updates the color palette for v3 guidance day/night styles.
v2 guidance map styles will still be used by default but convenient methods for any version has been added in the form of (MGLStyle.navigationGuidance{Day/Night}StyleURL(version:).

@1ec5 @bsudekum 馃憖

lineCasing.lineOpacity = NSExpression(forConstantValue: 0.9)

return lineCasing
func alternateRouteStyleLayer(identifier: String, lineColor: UIColor, lineWidth: NSExpression, source: MGLSource) -> MGLStyleLayer {
Copy link
Contributor

Choose a reason for hiding this comment

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

What are we gaining by passing through line color/width?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, the alternate route didn't have a casing before so this method is now used for creating the alternate route style layer and the casing layer.

@bsudekum
Copy link
Contributor

bsudekum commented May 3, 2018

image
simulator screen shot - iphone x - 2018-05-03 at 12 17 55

I'm slightly concerned with the contrast here. Now since the strongest color here is white, we do not get such an easy division between ui elements and the map style.

/cc @aparlato @cjballard @1ec5 @brsbl

@1ec5
Copy link
Contributor

1ec5 commented May 3, 2018

The second screenshot demonstrates that we should dim the rest of the screen when the end-of-route feedback view controller comes into view. But I think that would need to be a separate PR focused on the end-of-route experience.

/cc @JThramer

@bsudekum
Copy link
Contributor

bsudekum commented May 3, 2018

I'm going to remove this issue from the v0.17.0 milestone as there seems to be some kinks to work out with the style/ui still.

@Cjballard-zz
Copy link

Exploring a few options for increasing contrast here. We could style the top banner to be Mapbox gray-blue (#273D56) to provide some weight and color at the top of the view.

banner-darker

We could style the sub-banners as either slightly darker or slightly lighter to make it feel connected 鈥撀營 prefer the slightly darker color, but want to crowdsource some thoughts. From there, tweaked a little bit of spacing and included a 1pt light-gray (#C6D2E1) top border on the bottom tray to provide some contrast with the map style. Just some quick exploring - thoughts? /cc @bsudekum @frederoni @1ec5

banner-comparison

@bsudekum
Copy link
Contributor

bsudekum commented May 4, 2018

@cjballard noting, we have two styles: day and night. Darker tones are usually reserved for the night style/UI.

@Cjballard-zz
Copy link

@bsudekum Definitely. Don't mean to infringe on night mode, but explore how we can put some contrast & color in the current day theme UI. Mapbox core blue felt a little too Google-blue in this context, but I can experiment with some other options.

@Cjballard-zz
Copy link

An easier solution without changing banner color could be adding a 1pt light-gray (#e2e2e2 or whichever our preferred light gray we already use) line to the top of each trip tray & arrival card element to create some distinction. It's still light, but starts to create separation b/w elements. @bsudekum

banner-lighter-style

@brsbl-zz
Copy link

brsbl-zz commented May 8, 2018

How about we use #F2F5FA or #BCCBDE per our tertiary brand palette?
b4db1876-290b-11e7-8bdf-7b7476e1225e

@frederoni
Copy link
Contributor Author

frederoni commented May 8, 2018

Made a few updates here using colors from the brand palette.
@cjballard should these lines be 1px instead of 1pt?

@Cjballard-zz
Copy link

@frederoni Yeah, especially with the light gray/blue that's looking a little thick.

@frederoni
Copy link
Contributor Author

frederoni commented May 8, 2018

1px line

@Cjballard-zz
Copy link

@frederoni Much better, looks good to me.

@frederoni
Copy link
Contributor Author

@bsudekum @vincethecoder this PR is ready for review

Copy link
Contributor

@bsudekum bsudekum left a comment

Choose a reason for hiding this comment

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

I'd like to put a hold on this PR for a moment. I'm not sure if the new styles and UI add value to our customers. The lack of contrast in the style is still an issue for me.

Copy link
Contributor

@bsudekum bsudekum left a comment

Choose a reason for hiding this comment

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

@frederoni looks good, want to add a changelog entry with the new style-able classes for developers who are using a custom Style?

Copy link
Contributor

@vincethecoder vincethecoder left a comment

Choose a reason for hiding this comment

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

LGTM 馃憤

@frederoni
Copy link
Contributor Author

want to add a changelog entry with the new style-able classes

the SeparatorView is actually old so there are no new stylable classes but I added a blurb about the new convenient methods on MGLStyle.

@frederoni frederoni merged commit 1f52631 into master May 9, 2018
@frederoni frederoni deleted the fred/nav-v3-colors branch May 9, 2018 17:05
@1ec5
Copy link
Contributor

1ec5 commented Aug 10, 2018

Note that, despite the title of this PR, navigationGuidanceDayStyleURL and navigationGuidanceNightStyleURL still point to the v2 styles. #1347 continues to track upgrading to the v3 styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to v4 navigation map styles
6 participants