search

mapbox / mapbox-navigation-ios

Turn-by-turn navigation logic and UI in Swift on iOS
https://docs.mapbox.com/ios/navigation/
Other
861 stars 310 forks source link

Proposal: Replace UIAppearance-based theming #1950

Closed macdrevx closed 2 years ago

macdrevx commented 5 years ago

Mapbox Navigation SDK version: 0.28.0

There have been a couple of issues relating to the theming feature in the SDK (#1540, #1738). In my comment on the former, I identified two lines where some view properties were set manually rather than relying on UIAppearance. Even in the latest version of the SDK, there are still occurrences of this happening. The problem is that doing so causes those properties on those instances to stop participating in UIAppearance such that workarounds like the one referenced by @frederoni here will not work.

I propose that we could make the theming support much more robust by choosing to replace the UIAppearance-based implementation with some other approach.

Here are some proposed requirements for the new solution:

  1. There should not be any need to add/remove portions of the view hierarchy when changing themes.
  2. It should be possible to set values on instances rather than on classes so that temporary customizations can be preserved when changing themes.
  3. The theming should be decentralized. Define a standard palette interface (colors, fonts, etc) that is implemented by each theme, but leave it up to individual classes to actually make selections from the palette.
  4. The design should allow the creation of custom themes so long as they implement the full palette interface (or customize an existing one via subclassing).
  5. Continue using UIAppearance for any styling that isn't theme-dependent.

Prior art:

We use https://github.com/jiecao-fm/SwiftTheme to accomplish similar functionality. While it's not perfect, I do like the usability of being able to set theme-based properties per instance like theme_backgroundColor, theme_tintColor.

frederoni commented 5 years ago

Thanks for a great proposal. A 3rd-party theme manager did cross our minds a few times, but we expected Apple to deliver instance based UIAppearance styling in iOS 12. ☹️ I haven't looked into SwiftTheme specifically, but I think an instance based theme manager is the way to go.

macdrevx commented 5 years ago

@frederoni I'd be open to taking a stab at an implementation. I'd lean toward writing a custom instance-based theme manager. Let me know if you're open to it and if you have any other goals to add to the ones I mentioned.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

Stale.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

Stale.