Skip to content

Handle connection lost error on the DailyChallengeView#179

Open
mltbnz wants to merge 9 commits intopointfreeco:mainfrom
mltbnz:add-offline-error-to-dailyChallenge
Open

Handle connection lost error on the DailyChallengeView#179
mltbnz wants to merge 9 commits intopointfreeco:mainfrom
mltbnz:add-offline-error-to-dailyChallenge

Conversation

@mltbnz
Copy link
Copy Markdown

@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