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

Fix/ABW-957 Biometrics check cancelled causes blank screen #295

Merged
merged 4 commits into from
Feb 14, 2023

Conversation

nikola-milicevic
Copy link
Contributor

Description

This PR fixes the bug when user is presented blank screen after cancelling biometrics check upon account / persona creation.
In case of persona creation the flow can be dismissed, since it's modally presented, but in account creation flow the user gets stuck and the only option is to force quit the app.

Slack thread: https://rdxworks.slack.com/archives/C03QFAWBRNX/p1676379721309619

Copy link
Contributor

@davdroman-rdx davdroman-rdx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid. Well done 🙌

Comment on lines 46 to 48
return .run { send in
await send(.delegate(.biometricsCheckFailed))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return .run { send in
await send(.delegate(.biometricsCheckFailed))
}
return .send(.delegate(.biometricsCheckFailed))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wholeheartedly endorse this suggested change...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also just became Point-Free canon yesterday https://www.pointfree.co/episodes/ep222-composable-navigation-tabs

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unresolved for the rest of the team's visibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also heard that last night. If they are serious about it, I wonder if they couldn't provide a separate pathway for delegate actions, so we don't need to manually ignore them in the child reducer, and also don't have to go full nesting with .view...

Copy link
Contributor

@davdroman-rdx davdroman-rdx Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well at least we have FeatureReducer now which ignores them by default. But I do agree if it becomes a widespread pattern it might be worth protocolizing it like they did with BindingAction... though seeing how involved that API ended up in practice it might just be best to keep it as-is.

…/CreationOfEntity+Reducer.swift

Co-authored-by: David Roman <116723827+davdroman-rdx@users.noreply.github.com>
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikola-milicevic nikola-milicevic merged commit 9637d58 into main Feb 14, 2023
@nikola-milicevic nikola-milicevic deleted the fix/ABW-957-biometrics-check-cancelled branch February 14, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants