StanfordSpezi / SpeziViews

A common set of SwiftUI views and related functionality used in Spezi modules
https://swiftpackageindex.com/StanfordSpezi/SpeziViews/documentation/
MIT License
4 stars 3 forks source link

Implement a generic AsyncButton with support for throwing closures #8

Closed Supereg closed 1 year ago

Supereg commented 1 year ago

Implement a generic AsyncButton with support for throwing closures

:recycle: Current situation & Problem

Currently, Spezi Views (nor SwiftUI) provide a way to easily manage buttons with asynchronous action closures. We have several use cases where a reusable button with an async (throwing) action closure would be helpful (e.g. SpeziAccount for Login/Signup/Password Reset buttons).

:bulb: Proposed solution

This PR introduces a the new, reusable component AsyncButton that allows to supply async and async throwing action closures. It handles disabling the button and rendering of a processing indicator automatically. To deal with thrown errors of the action closure, we reuse the ViewState abstraction already available in SpeziViews. This can easily be combined with the viewStateAlert(state:) modifier to display a simple error alert.

:gear: Release Notes

:heavy_plus_sign: Additional Information

Related PRs

This PR refines the AsyncDataEntrySubmitButton implementation currently used in the upcoming SpeziAccount PR https://github.com/StanfordSpezi/SpeziAccount/pull/7 and moves it to SpeziViews.

Testing

Additional UI tests were added.

Reviewer Nudging

Ideally, have a look at the added UI tests and then at the AsyncButton implementation itself. It might be helpful to play around with the Previews of AsyncButton as well.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Supereg commented 1 year ago

@PSchmiedmayer SpeziViews seems to have an outdated list of required status checks in the branch protection rule.

PSchmiedmayer commented 1 year ago

Fixed the merge check requirements @Supereg 👍

Supereg commented 1 year ago

Fixed the merge check requirements @Supereg 👍

The Upload Coverage Report action is still referencing the old one. If you could update that as well 😇

codecov[bot] commented 1 year ago

Codecov Report

Merging #8 (11beb6d) into main (bae5330) will decrease coverage by 1.36%. The diff coverage is 72.23%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8/graphs/tree.svg?width=650&height=150&src=pr&token=7UbLX71dAO&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi)](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) ```diff @@ Coverage Diff @@ ## main #8 +/- ## ========================================== - Coverage 78.11% 76.74% -1.36% ========================================== Files 19 22 +3 Lines 612 748 +136 ========================================== + Hits 478 574 +96 - Misses 134 174 +40 ``` | [Impacted Files](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) | Coverage Δ | | |---|---|---| | [...eziViews/Environment/DefaultErrorDescription.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aVZpZXdzL0Vudmlyb25tZW50L0RlZmF1bHRFcnJvckRlc2NyaXB0aW9uLnN3aWZ0) | `100.00% <ø> (ø)` | | | [...Views/Environment/ProcessingDebounceDuration.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aVZpZXdzL0Vudmlyb25tZW50L1Byb2Nlc3NpbmdEZWJvdW5jZUR1cmF0aW9uLnN3aWZ0) | `50.00% <50.00%> (ø)` | | | [Sources/SpeziViews/Views/Button/AsyncButton.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aVZpZXdzL1ZpZXdzL0J1dHRvbi9Bc3luY0J1dHRvbi5zd2lmdA==) | `70.80% <70.80%> (ø)` | | | [...es/SpeziViews/ViewModifier/ProcessingOverlay.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aVZpZXdzL1ZpZXdNb2RpZmllci9Qcm9jZXNzaW5nT3ZlcmxheS5zd2lmdA==) | `76.48% <76.48%> (ø)` | | | [Sources/SpeziViews/Model/ViewState.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aVZpZXdzL01vZGVsL1ZpZXdTdGF0ZS5zd2lmdA==) | `97.83% <100.00%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Last update [bae5330...11beb6d](https://app.codecov.io/gh/StanfordSpezi/SpeziViews/pull/8?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi).
Supereg commented 1 year ago

@PSchmiedmayer Do you remember how we fixed the issue of a 0% code coverage 🙈

EDIT: nevermind. Codecov was still outdated.

Supereg commented 1 year ago

Was there an easy way to exclude PreviewProviders from code coverage reporting?

PSchmiedmayer commented 1 year ago

Not really, that is an unfortunate thing that Xcode does not really support. I have tried a lot of things with a dedicated TEST build configuration in the Template App but this wasn't ideal either and results in large build times when switching between DEBUG and TEST builds ... I can merge the PR like this.

Regarding code coverage: Sometimes Xcode is flaky with the test reporting. If that happens, I have made a good experience adding the target to the build scheme for tests (e.g. SpeziViews in this case):

Screenshot 2023-07-13 at 4 19 37 PM

And then explicitly adding it in the coverage report settings:

Screenshot 2023-07-13 at 4 20 04 PM
Supereg commented 1 year ago

Thanks for the tipps regarding code coverage. feel free to merge 👍