[Guidance Cards][Part 1] Making Guidance Cards controls stylable #2401

nishant-karajgikar · 2020-06-15T15:52:28Z

This PR begins adding support for styling the various labels, controls and views in the guidance card framework.

It does NOT hook up this new styling logic to the Guidance Cards framework (yet). That will be tackled in a follow up PR.

Because guidance cards are highlight-able, this PR adds highlight states to the various labels/controls.

nishant-karajgikar · 2020-06-18T16:27:20Z

This CI failure seems to be an unrelated bug - #2394

avi-c

This generally looks good to me. I can't check it out with Apex at the moment due to version conflicts with all the changes to MapboxDirections & MapboxNavigation, but I will try it as soon as I can.

For now, I would ask if testing has been done to verify that setting a custom value for these colors does indeed change them, and if the rendering responds correctly to Dark/Light mode changes for color assets. If those have been checked then I am ok with this being merged.

avi-c · 2020-06-25T17:58:43Z

MapboxNavigation/InstructionsCardContainerView.swift

@@ -66,6 +66,9 @@ public class InstructionsCardContainerView: UIView {
    private var secondaryChildren: [UIView] {
        return [lanesView, nextBannerView]
    }
+
+    @objc dynamic public var customBackgroundColor: UIColor = #colorLiteral(red: 1.0, green: 1.0, blue: 1.0, alpha: 1.0)


What is this customBackgroundColor used for?

The previous InstructionsCardStyle protocol called this option backgroundColor; it controlled the normal background of an unhighlighted card. Did we end up calling it something different to avoid dealing with property overloading in a subclass? I’m not sure this property works; setGradientLayer(for:) still calls backgroundColor on the style:

mapbox-navigation-ios/MapboxNavigation/InstructionsCardContainerView.swift

Line 141 in d0536ea

let backgroundColor = style.backgroundColor

Ah, never mind, #2402 takes care of using customBackgroundColor. If overloading backgroundColor is a problem, then I think normalBackgroundColor would be more intuitive in conjunction with the other highlightedBackgroundColor property.

#2402 has been merged into this PR, so the property is now used. I think it would be great if we could take care of renaming the property to normalBackgroundColor before merging.

…e to UIAppearance proxy.

1ec5 · 2020-08-03T22:05:05Z

MapboxNavigation/InstructionsCardContainerView.swift

@@ -66,6 +66,9 @@ public class InstructionsCardContainerView: UIView {
    private var secondaryChildren: [UIView] {
        return [lanesView, nextBannerView]
    }
+
+    @objc dynamic public var customBackgroundColor: UIColor = #colorLiteral(red: 1.0, green: 1.0, blue: 1.0, alpha: 1.0)


The previous InstructionsCardStyle protocol called this option backgroundColor; it controlled the normal background of an unhighlighted card. Did we end up calling it something different to avoid dealing with property overloading in a subclass? I’m not sure this property works; setGradientLayer(for:) still calls backgroundColor on the style:

mapbox-navigation-ios/MapboxNavigation/InstructionsCardContainerView.swift

Line 141 in d0536ea

let backgroundColor = style.backgroundColor

1ec5 · 2020-08-03T22:07:25Z

MapboxNavigation/InstructionsCardContainerView.swift

@@ -47,7 +47,7 @@ public extension InstructionsCardContainerViewDelegate {
 }

 /// :nodoc:


We’re going to need documentation for this class and its properties as we turn guidance cards into an officially supported feature.

…nce Card styling.

…13.5 since iPhone 6 Plus is not supported in that OS but we test for it in CI.

avi-c · 2020-08-04T21:07:04Z

@1ec5 - I believe we can merge this now. I opened #2519 to track the eventual documentation work for guidance card styling.

If you agree can you approve it?

…arance proxy based styling of Guidance Cards.

…y for now.

[Guidance Cards][Part 4] Remove InstructionsCardStyle class

[Guidance Cards][Part 2]Defer to UIAppearance proxy for color/font literals

1ec5

Looks good, other than renaming customBackgroundColor to normalBackgroundColor. We’ll also need to track removing the :nodoc:s and formally documenting the new API as tail work.

1ec5 · 2020-08-04T22:06:59Z

MapboxNavigation/InstructionsCardContainerView.swift

@@ -66,6 +66,9 @@ public class InstructionsCardContainerView: UIView {
    private var secondaryChildren: [UIView] {
        return [lanesView, nextBannerView]
    }
+
+    @objc dynamic public var customBackgroundColor: UIColor = #colorLiteral(red: 1.0, green: 1.0, blue: 1.0, alpha: 1.0)


#2402 has been merged into this PR, so the property is now used. I think it would be great if we could take care of renaming the property to normalBackgroundColor before merging.

nishant-karajgikar requested a review from 1ec5 June 15, 2020 18:22

nishant-karajgikar mentioned this pull request Jun 15, 2020

[Guidance Cards][Part 2]Defer to UIAppearance proxy for color/font literals #2402

Merged

nishant-karajgikar changed the title ~~Making Guidance Cards controls stylable~~ [Guidance Cards][Part 1] Making Guidance Cards controls stylable Jun 15, 2020

nishant-karajgikar force-pushed the nishantk/styleGuidanceCards branch 2 times, most recently from cecfd88 to c47b8e8 Compare June 16, 2020 16:05

nishant-karajgikar requested a review from avi-c June 18, 2020 16:27

fabian-guerra assigned nishant-karajgikar Jun 24, 2020

avi-c reviewed Jun 25, 2020

View reviewed changes

1ec5 added this to the v1.0.0 milestone Jul 2, 2020

nishant-karajgikar added 3 commits August 3, 2020 11:50

Making controls stylable

afc364f

Fix Stylable label logic

61378ed

Make Next banner stylable

d0536ea

avi-c force-pushed the nishantk/styleGuidanceCards branch from c47b8e8 to d0536ea Compare August 3, 2020 18:52

avi-c changed the base branch from master to release-v1.0-pre-registry August 3, 2020 18:52

nishant-karajgikar and others added 2 commits August 3, 2020 13:24

Defer to UIAppearance proxy for color/font literals

2d00fff

Move cornerRadius property of InstructionsCardContainerView from Styl…

e75fbf1

…e to UIAppearance proxy.

1ec5 reviewed Aug 3, 2020

View reviewed changes

1ec5 added the op-ex Refactoring, Tech Debt or any other operational excellence work. label Aug 3, 2020

avi-c added 2 commits August 4, 2020 12:49

Regenerated UI test snapshot fixtures to match new defaults for Guida…

692e865

…nce Card styling.

Generate test fixture assets for guidance cards iPhone 8 Plus on iOS …

f9fcde7

…13.5 since iPhone 6 Plus is not supported in that OS but we test for it in CI.

avi-c mentioned this pull request Aug 4, 2020

Publicly document guidance cards API (and fix naming convention issues) #2519

Open

avi-c and others added 4 commits August 4, 2020 14:12

Remove references to InstructionsCardStyle to complete move to UIAppe…

469ecc9

…arance proxy based styling of Guidance Cards.

Remove Guidance Card custom styling class tests. Rely on UI tests onl…

6dd9d8d

…y for now.

Merge pull request #2510 from mapbox/avic/styleGuidanceCardsPt4

6b05d8a

[Guidance Cards][Part 4] Remove InstructionsCardStyle class

Merge pull request #2402 from mapbox/nishantk/styleGuidanceCardsPt2

5281311

[Guidance Cards][Part 2]Defer to UIAppearance proxy for color/font literals

1ec5 approved these changes Aug 4, 2020

View reviewed changes

avi-c merged commit 0fb3593 into release-v1.0-pre-registry Aug 4, 2020

avi-c deleted the nishantk/styleGuidanceCards branch August 4, 2020 22:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Guidance Cards][Part 1] Making Guidance Cards controls stylable #2401

[Guidance Cards][Part 1] Making Guidance Cards controls stylable #2401

nishant-karajgikar commented Jun 15, 2020

nishant-karajgikar commented Jun 18, 2020

avi-c left a comment

avi-c Jun 25, 2020

1ec5 Aug 3, 2020

1ec5 Aug 3, 2020

1ec5 Aug 4, 2020

1ec5 Aug 3, 2020

1ec5 Aug 3, 2020

avi-c commented Aug 4, 2020

1ec5 left a comment

1ec5 Aug 4, 2020

		@@ -47,7 +47,7 @@ public extension InstructionsCardContainerViewDelegate {
		}

		/// :nodoc:

[Guidance Cards][Part 1] Making Guidance Cards controls stylable #2401

[Guidance Cards][Part 1] Making Guidance Cards controls stylable #2401

Conversation

nishant-karajgikar commented Jun 15, 2020

nishant-karajgikar commented Jun 18, 2020

avi-c left a comment

Choose a reason for hiding this comment

avi-c Jun 25, 2020

Choose a reason for hiding this comment

1ec5 Aug 3, 2020

Choose a reason for hiding this comment

1ec5 Aug 3, 2020

Choose a reason for hiding this comment

1ec5 Aug 4, 2020

Choose a reason for hiding this comment

1ec5 Aug 3, 2020

Choose a reason for hiding this comment

1ec5 Aug 3, 2020

Choose a reason for hiding this comment

avi-c commented Aug 4, 2020

1ec5 left a comment

Choose a reason for hiding this comment

1ec5 Aug 4, 2020

Choose a reason for hiding this comment