search

codeforbtv / cvoeo-app

The "Money on My Mind" app helps CVOEO's Reach-Up clients stay on track with their personal finance coaching.
Apache License 2.0
11 stars 4 forks source link

Implemented Logout circular menu from ellipses #95

Closed jfenner closed 5 years ago

jfenner commented 5 years ago

This is my first change for CodeForBTV, so making sure that I'm following protocol correctly.

I did not implement this as Material UI, as I found some library conflicts with the version of react that the MOMM app is using and what Material UI needed. I just used the react native Animation.View.

Let me know if there is anything that you would do different or a better practice.

Implementation of https://github.com/codeforbtv/cvoeo-app/issues/94

doub1ejack commented 5 years ago

@jfenner Unfortunately I won't be able to pull & test this today, but from a quick skim on my phone the code looks nice. I'm excited to watch the magic happen.

@nfloersch & @jbpayne, Can either of you review?

@jfenner One comment I have is regarding whitespace. Until we get #41 fixed, the diffs will have a lot of whitespace clutter that make it more challenging to do reviews. It can be helpful to others if the authors add comments that highlight the big logic changes among the whitespace noise.

Thanks for the PR! We'll turn this around soon.

jfenner commented 5 years ago

OK, I'm stuck on the Android side. Here's what I'm seeing: The logout circle expands and contracts as designed. However, I can't get the actual Logout button to be pressed. You can press on the area that is part of the top header when the logout circle is expanded, but it doesn't seem to be able to pickup with click where the actual logout text is. Frustrating, because it works fine on iOS, and everything I've read online seems to say it should work.

Thoughts, ideas? We can merge as is if you want as it is partially working and open up another issue to fix the Android side.

Let me know... thanks!