search

shaps80 / SwiftUIBackports

A collection of SwiftUI backports for iOS, macOS, tvOS and watchOS
MIT License
931 stars 59 forks source link

Reset `tintAdjustmentMode` for `presentationUndimmed` #22

Closed schiewe closed 1 year ago

schiewe commented 1 year ago

Have you read the Contributing Guidelines?

Yes! 🎉

Issue #

Describe your changes

As discussed here: https://danielsaidi.com/blog/2022/06/21/undimmed-presentation-detents-in-swiftui tintAdjustmentMode should be set to .normal in order to "undimm" the underlying view otherwise it stays dimmed even when the sheet is dismissed (at least in a NavigationView context).

shaps80 commented 1 year ago

Nice catch!

shaps80 commented 1 year ago

This is now in release v1.7.0 👍 – thanks for the contribution ;)

FYI: Your implementation had issues that I didn't notice until I tested it more completely. I've decided on a new API altogether.

presentationUndimmed is now deprecated in favour of: presenatationDetents(_:selection:largestUndimmedDetent:)

This basically keeps all the detent API together and allowed me to properly resolve the dimming issue. The interactive Demo project can be used to see it in action, just toggle the passthrough switch :)

schiewe commented 1 year ago

@shaps80 Nice work! 💪 Thanks for the update. What were the issues that you encountered with the 1.7.0 solution?

I see some issues with the 1.8.2 API regarding compatibility with the original backported iOS 16 API to have this library a drop-in solution:

  1. presentationDetents(_ detents:) sets largestUndimmed: .large which is not the iOS 16 default. iOS 16 does not undim the underlying view at all. I think this was the reason presentationUndimmed(from:) was introduced in the first place. If you want to go this route I'd probably also add an optional largestUndimmedDetent parameter to this modifier default initialized with nil as for presentationDetents(_:selection:largestUndimmedDetent:).
  2. Docs missing for the largestUndimmedDetent parameter of presentationDetents(_:selection:largestUndimmedDetent:).
  3. I am not sure if it is a good idea in general to change the original API that is intended to be backported even with additional optional arguments. I'd prefer to use the original API if available. Having the additional options beyond the scope of the original backported API in a separate modifier makes it easier to use the additional functionality in combination with the original API, e.g., using .backport.presentationUndimmed(from:) together with the non-backported version of .presentationDetents(_:) even when branched directly in .backport.presentationDetents(_:). I am also planning to expose prefersScrollingExpandsWhenScrolledToEdge so that it can be set to false...
shaps80 commented 1 year ago
  1. presentationDetents(_ detents:) sets largestUndimmed: .large which is not the iOS 16 default.

This is true but when it's in the large position that value is ignored anyway and this approach avoided an unnecessary optional. It works fine afaict but if you come across a bug I would happily change it. So far seems to work well across all iOS versions for me.

  1. Docs missing for the largestUndimmedDetent parameter of presentationDetents(_:selection:largestUndimmedDetent:).

Nice catch! Will add 👍

For your last point about changing original API I'll create a dedicated issue so we can discuss in more detail.

https://github.com/shaps80/SwiftUIBackports/issues/26