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

[Feat] keep track of previously logged out users #2906

Merged
merged 7 commits into from
Aug 16, 2024

Conversation

aliseyalvi
Copy link
Collaborator

@aliseyalvi aliseyalvi commented Aug 3, 2024

What does this PR?

This PR keep track of previously logged out users and show them in account bottom sheet for easy login.
User is redirected to login screen with prefilled username

Issue number

fixes #2904

Screenshots/Video

Monosnap.screencast.2024-08-03.13-00-23.mp4

* show logged out users in account bottom sheet to prefill the username when login
Copy link
Collaborator

@noumantahir noumantahir left a comment

Choose a reason for hiding this comment

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

left some review comments, functionality wise, seems good.

@@ -55,7 +80,7 @@ const AccountsBottomSheet = forwardRef(
onClose={onClose}
>
<FlatList
data={accounts}
data={[...accounts, ...prevLoggedInUsers]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

to keep data flow more organised, why not added a section below list instead of mixing different objects? can even add a section labeled Logged out accounts

<Text style={styles.name}>{`@${item.username}`}</Text>
const _handlePressAccountTile = (item) => {
if (
item &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

this huge logic can be simplified using ? and ! operators.

@aliseyalvi
Copy link
Collaborator Author

@noumantahir PR updated according to review

  • added option to remove user from list
  • added logged out users to separate sections
Monosnap.screencast.2024-08-10.20-15-21.mp4

@aliseyalvi
Copy link
Collaborator Author

@noumantahir updated icon
image

Copy link
Collaborator

@noumantahir noumantahir left a comment

Choose a reason for hiding this comment

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

One corner case that needs handling is, if I logout of every account and try to login again, there is no way to select from logged out list. it directly lands login screen.

My suggestion would be to open account switch modal at-least at these two places. or only open account switcher if there is at-least one logged out user.

Screenshot 2024-08-13 at 15 44 10 Screenshot 2024-08-13 at 15 43 57

@@ -72,6 +79,7 @@ interface State {
hidePostsThumbnails: boolean;
isTermsAccepted: boolean;
isBiometricEnabled: boolean;
prevLoggedInUsers: PrevLoggedInUsers[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if this the right place for preLoggedInUsers, for one application store has already overflown, and second since this is more related to account, perhaps it should be in accountReducer

Screenshot 2024-08-13 at 15 39 30

@aliseyalvi
Copy link
Collaborator Author

@noumantahir PR updated as per review.

  • show account switch modal if user is logged out
  • move prev logged out users data from application store to account store

@noumantahir
Copy link
Collaborator

lgtm

@feruzm feruzm merged commit 42dbc15 into development Aug 16, 2024
@feruzm feruzm deleted the sa/save-loggedin-users branch August 16, 2024 11:28
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.

keep track of previously logged out users
3 participants