Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Map view should not force its tint color onto subviews #9597

Open
friedbunny opened this issue Jul 24, 2017 · 6 comments
Open

Map view should not force its tint color onto subviews #9597

friedbunny opened this issue Jul 24, 2017 · 6 comments
Labels
bug gl-ios iOS Mapbox Maps SDK for iOS SEMVER-MAJOR Requires a major release according to Semantic Versioning rules

Comments

@friedbunny
Copy link
Contributor

friedbunny commented Jul 24, 2017

Platform: iOS v3.6.0

Since forever, we’ve been forcefully updating MGLMapView’s subviews to match its tint color. @boundsj’s post in #8522 (comment) contains a great explanation of why and how we’ve done this.

The problem is, it’s not necessary for MGLMapView to handle updating its subviews — UIKit will automatically inform subviews via -tintColorDidChange, where that subview can decide for itself whether or not to update with its parent view.

Proposal

  • Get rid of -[MGLMapView tintColorDidChange] entirely.
  • Implement -tintColorDidChange in custom subviews that we want to keep in sync with the map view’s tint — mainly MGLFaux3DUserLocationAnnotationView, perhaps MGLCompactCalloutView.
  • Recommend that developers do the same.

This would be a potentially breaking change for developers who are relying on not having to implement -tintColorDidChange themselves, so I’ve tentatively put it on the v3.7.0 milestone.

Example

  1. Set the attribution button’s tint color to a novel color.
  2. Present an alert view controller, which dims the map view tint color.
  3. Dismiss the alert view controller.

Current result: The attribution button’s custom tint color is lost, as MGLMapView recursively resets its subviews’ tint colors to match itself, triggered by -tintColorDidChange.

Fixed result: The attribution button’s custom tint color is kept, because it no longer inherits its tint from MGLMapView and we’re not forcing it to change.

/cc @1ec5 @boundsj @fabian-guerra @jmkiley

@friedbunny friedbunny added bug iOS Mapbox Maps SDK for iOS labels Jul 24, 2017
@friedbunny friedbunny added this to the ios-v3.7.0 milestone Jul 24, 2017
@friedbunny
Copy link
Contributor Author

The attribution button has been opted-out of MGLMapView’s tint updating (#9598), starting in v3.6.1.

@boundsj
Copy link
Contributor

boundsj commented Sep 18, 2017

Fixed in #9598

@boundsj boundsj closed this as completed Sep 18, 2017
@friedbunny
Copy link
Contributor Author

#9598 did this for the attribution button, but I think we should do the same for any subview of the map view.

@friedbunny friedbunny reopened this Sep 18, 2017
@boundsj boundsj removed this from the ios-v3.7.0 milestone Sep 18, 2017
@stale stale bot added the archived Archived because of inactivity label Nov 4, 2018
@stale
Copy link

stale bot commented Dec 2, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Dec 2, 2018
@julianrex julianrex reopened this Dec 3, 2018
@stale stale bot removed the archived Archived because of inactivity label Dec 3, 2018
@stale stale bot added the archived Archived because of inactivity label Jun 1, 2019
@stale
Copy link

stale bot commented Jun 1, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Jun 1, 2019
@julianrex julianrex reopened this Jun 3, 2019
@stale stale bot removed the archived Archived because of inactivity label Jun 3, 2019
@friedbunny friedbunny added the SEMVER-MAJOR Requires a major release according to Semantic Versioning rules label Jun 3, 2019
@jmkiley jmkiley added the gl-ios label Nov 21, 2019
@gobetti
Copy link

gobetti commented May 29, 2020

Hey team, sorry for summoning this back but, this is actually causing our whole callout (and subviews) tintColor to be replaced, and looking at the -[MGLMapView updateTintColorForView:] loop I'm not seeing a way around it.

Do you still have plans to remove that? 🙏

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug gl-ios iOS Mapbox Maps SDK for iOS SEMVER-MAJOR Requires a major release according to Semantic Versioning rules
Projects
None yet
Development

No branches or pull requests

5 participants