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

add logout functionality #40

Closed
housseindjirdeh opened this issue Jul 13, 2017 · 6 comments
Closed

add logout functionality #40

housseindjirdeh opened this issue Jul 13, 2017 · 6 comments

Comments

@housseindjirdeh
Copy link
Member

So we can't logout of the app right now :). Currently users have to delete the app and redownload in order to sign in with another account.

We can just navigate the user to the initial welcome screen (might be worth clearing our persisted store too).

@alejandronanez
Copy link
Member

Any steps for the new people to the repo to follow? Also, any thoughts about the design? I think a logout link in the profile page would be enough.

@housseindjirdeh
Copy link
Member Author

Thank you @alejandronanez, I had it all in mind and just realized I didn't actually write it down 😞 .

So the UserProfile screen has a menu icon on the top right that lets you follow/unfollow users. We don't have this for the AuthUserProfile screen and I think we can just add this to open an Action Sheet with a Logout button.

2 caveats:

  1. ActionSheetIOS is being used and will be changed soon: Android Version #2 (comment) to allow for Android functionality. Will appreciate if we use react-native-actionsheet here as well
  2. There's another ticket open (Update profile details #8) to allow editing user profile details. If that gets merged in prior --> it probably will make more sense to have Log Out included in that screen. But in the meantime the approach mentioned above will work fine

@housseindjirdeh housseindjirdeh added this to To Do in Roadmap Jul 14, 2017
@RolfKoenders
Copy link
Member

I started to play with logging out a bit and at the moment i have the following.

Screen_Shot_2017_07_17_at_20_31_15 Screen_Shot_2017_07_17_at_20_31_25

By clicking yes you would navigate to the 'Login' screen. At this moment i'm trying to figure out how to clear the userData (log the user out).
Till there will be an editing button (Maybe a cogwheel icon next to the profile image will do?) this will do the job i think?

Let me know if i should push my branch and create a pr or its better to combine this with a possible editing feature.

@housseindjirdeh
Copy link
Member Author

housseindjirdeh commented Jul 17, 2017

@RolfKoenders that looks amazing 😍

Yep so I had the idea in another ticket #8 for updating profile details to have a little cog wheel icon to navigate to the screen where the user can update their info. I was thinking of having Log Out there but I actually really what you did there. I think this is great for now and when the user profile details screen is created --> we can decide if we want to just move the listitem there or leave it where it is now.

Couple of tiny tiny nitpicks I can see (and I know I could have waited for a PR to put this):

  1. Since the login screen there's a button for Sign In, maybe we should call this Sign Out instead of Log Out (I know - very nitpicky 😂 )
  2. Think it might look a little nicer if we leverage SectionList and put your new list item within it. SectionList title can also be called Sign Out
  3. Unsure but are you using octicons for the icon on the left like this. If you are, disregard this point 😂 but if not - definitely makes sense to leverage GitHub icons like we do everywhere in the app (feel free to pick any icon that you think makes sense - like an arrow or something)!
  4. Would probably say we should also use hideChevron as a prop (like this to hide the right chevron icon as well since we don't navigate to a separate page

Aside from these tiny things, this is awesome matey. More than happy to have you submit a PR with a dummy ListItem for now and I can show you exactly how we can clear user data if you like (should be really straightforward). Since we use redux-persist in the app to persist the entire user data to AsyncStorage, the idea is pretty much we use persistStore.purge or simply AsyncStorage.clear() when we navigate to the login screen

@RolfKoenders
Copy link
Member

As referenced above i created a pr and here are the other answers.

  1. Agree, these things give the overall feeling of the app.
  2. I thought it would look bad, but it isn't.
  3. This is a octicons icon.
  4. I'm trying 😄

@Antoine38660
Copy link
Member

Can be closed (#131) 😄

@Antoine38660 Antoine38660 moved this from Release 1.2 to Done in Roadmap Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

5 participants