search

johnpatrickmorgan / FlowStacks

FlowStacks allows you to hoist SwiftUI navigation and presentation state into a Coordinator
MIT License
832 stars 65 forks source link

Fix sheet presentation in < iOS 14.5 #36

Closed alejandroruizponce closed 1 year ago

alejandroruizponce commented 2 years ago

Fix proposed to fix issues when presenting manners in versions prior to iOS 14.5.

Also added dismiss closures that were set to nil in the present function.

johnpatrickmorgan commented 2 years ago

Thanks for raising this PR @alejandroruizponce! I'm away for a few days so I haven't been able to check it out properly yet. In the meantime, please could you explain what specific issue this solves, so I can test it? Thanks!

johnpatrickmorgan commented 2 years ago

Also, on first reading, it seems like the proposed change would stop presentCover from working on iOS 14 and below?

alejandroruizponce commented 2 years ago

Also, on first reading, it seems like the proposed change would stop presentCover from working on iOS 14 and below?

Hi @johnpatrickmorgan , sorry for the time without replying, I have not been available either. Yes, there is an issue with this line in <iOS 14.5: https://github.com/johnpatrickmorgan/FlowStacks/blob/e4f28ed9de20e1c01302d498b107b4e1bf095cb3/Sources/FlowStacks/Node.swift#L114

Surely by how they are treated the presentation of manners in < iOS 14.5 is different. When presenting a sheet this line is launching several times and returning false on the last one. Which results in not presenting a sheet but a cover.

Taking into account how deprecated iOS 13 is I thought it was a better option to leave the present only for sheets and thus avoid this bug. By removing that bug and adding the backport extension, the sheets are displayed properly in both iOS 14 and 15.

johnpatrickmorgan commented 1 year ago

I'm still not too clear what issue this PR addresses, so I'll close it for now.