wwt / SwiftCurrent

A library for managing complex workflows in Swift
https://wwt.github.io/SwiftCurrent/
Apache License 2.0
307 stars 19 forks source link

Adds support for Full screen cover modal presentation style #121

Closed Richard-Gist closed 3 years ago

Richard-Gist commented 3 years ago

Linked Issue: #119

Checklist:

codecov-commenter commented 3 years ago

Codecov Report

Merging #121 (9fd2f94) into main (e5150d4) will decrease coverage by 2.67%. The diff coverage is 65.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
- Coverage   94.31%   91.64%   -2.68%     
==========================================
  Files          77       79       +2     
  Lines        1883     2071     +188     
==========================================
+ Hits         1776     1898     +122     
- Misses        107      173      +66     
Impacted Files Coverage Δ
...tUIExample/SwiftUIExample/TestViews/TestView.swift 54.13% <54.13%> (ø)
...le/SwiftUIExample/TestViews/TestEnumerations.swift 86.84% <86.84%> (ø)
...ftUIExample/SwiftUIExample/SwiftUIExampleApp.swift 100.00% <100.00%> (ø)
...iftUI/Extensions/SwiftUILaunchStyleAdditions.swift 100.00% <100.00%> (ø)
...rces/SwiftCurrent_SwiftUI/Views/WorkflowItem.swift 94.77% <100.00%> (+0.16%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7cd7b21...9fd2f94. Read the comment docs.

Richard-Gist commented 3 years ago

EOD: Because fullScreenCover is untestable by ViewInspector, we will be adding our first XCUI tests. Additionally, fullScreenCover is not available in macOS, and I missed a step there that @Tyler-Keith-Thompson has probably already fixed before I can post this comment.

For the XCUI tests, you can see the last commits working towards an idea for how they will work. Ultimately I think where we want to go with it is that there are about 4 FRs that are shared code between the UI tests and SwiftUIExample, and they allow customization by taking in some argument that determines their shouldLoad. We then have a couple of views that are returned how FR1 is being returned now, but instead of just being a single FR1, they would return a view that has the WorkflowLauncher with the appropriate number of FR's and use the EnvironmentKey to customize aspects of the view appropriately.

This makes most things Enum driven to talk between the test and the app, and the code readily available between the two for development. Hopefully, we'll end up with about 4 FR views we use for testing and hopefully only about 4 or so views holding WorkflowLaunchers. We can do all the rest of our customizations from the environment we launch the app with.

Tyler-Keith-Thompson commented 3 years ago

EOD: We have XCUITests that work...they really are just horrible tests to have to write. We will stick to ViewInspector as much as possible. I am currently satisfied by the level of testing the XCUITests provide. However, until now they were not running in the pipeline. We may need to tweak them so that they can run in the pipeline because I might have gotten too fancy with ResultBuilders for Xcode 12.4...we'll see.

However, once our pipeline passes I am personally satisfied, let's ship out fullScreenCover.

NOTE: A SwiftUI bug: We discovered a frustrating SwiftUI bug where calling dismiss on presentationMode causes a bad animation and just general wonkiness when the navigation view is a column-based navigation view. I haven't the slightest idea why we are seeing that behavior, and I am not certain to what extent it is a problem. If people use embedInNavigationView() on WorkflowLauncher they'll be just fine. Xcode 13 does not have the same issue, so it's clear that it's not a SwiftCurrent issue.