Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Migrate to system API on iOS 13 #80

Closed
12 of 14 tasks
levinli303 opened this issue Jun 19, 2020 · 2 comments
Closed
12 of 14 tasks

Migrate to system API on iOS 13 #80

levinli303 opened this issue Jun 19, 2020 · 2 comments
Labels
enhancement New feature or request release blocker Something that we must fix before next release
Milestone

Comments

@levinli303
Copy link
Member

levinli303 commented Jun 19, 2020

Maybe should not implemented

  • A new place to modify current theme
  • Bridge classes that does not call super.traitCollectionDidChange(_:)

The first 6 is fixed in #78

@levinli303 levinli303 added enhancement New feature or request release blocker Something that we must fix before next release labels Jun 19, 2020
@levinli303
Copy link
Member Author

levinli303 commented Jul 2, 2020

Sharing some thoughts here. There are two issues that I'm considering.

  1. Is it really necessary to bridge classes that does not call super.traitCollectionDidChange(_:)

It is in the plan because if we don't bridge that, dmTraitCollectionDidChange(_:) won't be called when theme changes but now I'm kind of skeptical whether it is necessary.

I disassembled the UIKitCore (Went for the Catalyst one inside Mac (10.15.5) for convenience but there should not be a difference) and were able to find all these classes.

dmTraitCollectionDidChange(_:) is offered to users who wish to listen to and respond to theme changes in its subclasses on their own. So there are some classes we definitely don't need to bridge like private classes inside UIKit since you cannot subclass it easily. This already shortened the list of classes by 2/3. And there are classes which are not subclasses of UIView/UIViewController, apparently they can also be excluded (for now). The remaining ones are:

UIView subclasses: UIImageView, UILabel, UIDatePicker, UIPickerView
UIViewController subclass: UISplitViewController

My personal experience with UIKit's traitCollectionDidChange(_:) is that I usually use it in a UIViewController or UIView subclass that manages other view(s), so I can just modify its subviews' properties inside this method when trait collection changes. The four UIView subclasses don't fall into my category as they either don't usually have subviews or they manage subviews inside themselves. I can think of a use case for UIImageView subclass though

class MyImageView: UIImageView {
  override func dmTraitCollectionDidChange(_ previousTraitCollection: DMTraitCollection?) {
    super.dmTraitCollectionDidChange(previousTraitCollection)
    layer.borderColor = /* some code to retrieve a UIColor */.cgColor
  }
}

As for UISplitViewController, I'm not really familiar with it, but I think of it as a wrapper of multiple UIViewControllers and does not have any visual element of its own, so maybe not calling .dmTraitCollectionDidChange(_:) is ok?

also the list subjects to changes since these calls to super.traitCollectionDidChange(_:) are within Apple.

@levinli303
Copy link
Member Author

  1. We should add dmTraitCollection property requirement to DMTraitEnvironment just like UITraitEnvironment

I reflected on how I integrated IOS13's dark mode inside my own app when using traitCollectionDidChange(_:)

override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
  super.traitCollectionDidChange(previousTraitCollection)

  if self.traitCollection.userInterfaceStyle == .dark {
    /* configure the dark appearance */
  }
  else {
    /* configure the light appearance */
  }
}

Currently we don't have a self.dmTraitCollection on DMTraitEnvironment. Which means in order to find the current UIView(Controller)'s theme, one should:

  1. If on iOS 13+, use the one in UITraitEnvironment (since there might be overrideUserInterfaceStyle in these)
  2. else, use the one in DMTraitCollection.override

Since we are back porting it, we should just provide a self.dmTraitCollection that automatically does it for users.

What are your thoughts on these? @icodesign @zhuorantan @imWildCat @li-bei

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request release blocker Something that we must fix before next release
Projects
None yet
Development

No branches or pull requests

1 participant