Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for Full screen cover modal presentation style #121

Merged
merged 21 commits into from Sep 3, 2021

Conversation

Richard-Gist
Copy link
Collaborator

@Richard-Gist Richard-Gist commented Sep 1, 2021

Linked Issue: #119

Checklist:

Richard-Gist and others added 5 commits September 1, 2021 14:48
Co-authored-by: Tyler Thompson <tyler.thompson@wwt.com>
Co-authored-by: Tyler Thompson <tyler.thompson@wwt.com>
…ector tests to cover it - TT RAG

Co-authored-by: Tyler Thompson <tyler.thompson@wwt.com>
Co-authored-by: Tyler Thompson <tyler.thompson@wwt.com>
Co-authored-by: Tyler Thompson <tyler.thompson@wwt.com>
@Richard-Gist Richard-Gist added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 1, 2021
@Richard-Gist Richard-Gist added this to the SwiftUI Support milestone Sep 1, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2021

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%) ⬆️

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
Copy link
Collaborator Author

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.

… stuff working correctly, we will see what the pipeline says - TT RAG

Co-authored-by: Richard Gist <Richard.Gist@wwt.com>
…st for our SwiftCurrent_SwiftUI target...if Xcode will be cool about that - TT
…OS build, not super thrilled with how it shaped up, though - TT RAG

Co-authored-by: Richard Gist <Richard.Gist@wwt.com>
@Richard-Gist Richard-Gist linked an issue Sep 2, 2021 that may be closed by this pull request
@Tyler-Keith-Thompson
Copy link
Collaborator

Tyler-Keith-Thompson commented Sep 3, 2021

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.

@Tyler-Keith-Thompson Tyler-Keith-Thompson marked this pull request as ready for review September 3, 2021 00:37
@Tyler-Keith-Thompson Tyler-Keith-Thompson requested a review from a team as a code owner September 3, 2021 00:37
@morganzellers morganzellers changed the title Full screen cover Adds support for Full screen cover modal presentation style Sep 3, 2021
@morganzellers morganzellers merged commit 01f33ce into main Sep 3, 2021
@morganzellers morganzellers deleted the fullScreenCover branch September 3, 2021 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SwiftUI FullScreenCover support
4 participants