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

Handle connection lost error on the DailyChallengeView #179

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

mltbnz
Copy link

@mltbnz mltbnz commented May 19, 2023

Why the PR:
I tried to play the game in airplane mode and maybe didn't pay much attention where I tapped so I opened the DailyChallengeView. When I tapped either one of the two GameButtons nothing happened. I then realised I am offline I guess maybe would have expected some kind of message to tell me that I want to play a mode that expects a successful response from a network to set the state.

➕ Added

When the API request fails I try to map the code of the NSError to a FailureReason which gets set to the ApiError.
From there it gets passed to the state and actually just the failureReason of the Error gets pulled out to be exposed to the view components.

🧪 Experiment

I wanted to try out the waters I guess and see if this kind of functionality is something you'd consider adding to the codebase. The usage of String? to transport the errorMessage might not be ideal and maybe something like the FailureReason enum is better suited and could also communicate better what errors should be considered as gameblocking.

Let me know what you think

👀 Screenshots

Screenshot 2023-05-19 at 21 43 34 Screenshot 2023-05-19 at 21 43 41

mltbnz and others added 9 commits May 18, 2023 22:09
- add failure reason
- map NSError to failure reason
- add offline reason
- a test that checks for a failure response
- a test that checks if an alert state is set when gameButtonTapped action is invoced
# Conflicts:
#	Sources/SiteMiddleware/ServerEnvironment.swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants