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

[Nav Drawer] Move user profile and sign out link into popup #2397

Closed
gino-m opened this issue Mar 19, 2024 · 20 comments · Fixed by #2434
Closed

[Nav Drawer] Move user profile and sign out link into popup #2397

gino-m opened this issue Mar 19, 2024 · 20 comments · Fixed by #2434
Assignees
Labels
good first issue type: fr Request for new feature

Comments

@gino-m
Copy link
Collaborator

gino-m commented Mar 19, 2024

Specifically:

  • On tap of the user profile, show a dialog with the username and email, and two buttons as shown in the attached below. Note the placement of the dialog towards the top of the nav drawer.
  • If the user taps "Sign out", the exist warning dialog is shown asking the user to confirm.
  • If the user clicks "Cancel", the dialog is closed.
  • The existing "Sign out" nav drawer menu option can then be removed.

image

@gino-m gino-m added this to the 4. OSS launch (21-Jul-24) milestone Mar 19, 2024
@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

I'm excited about contributing to the Ground Android project and would like to work on issue #2397. I believe I have the skills necessary to contribute effectively to this task.

Before diving in, I wanted to ask if there are any specific guidelines or prerequisites I should be aware of. Also, if it's possible, could you please assign this issue to me to ensure we're not duplicating efforts? I'm also open to tackling other issues if you think there's a better place to start for a new contributor.

Looking forward to your guidance and contributing to the project!

Best,
Surya

@gino-m
Copy link
Collaborator Author

gino-m commented Mar 24, 2024

Hi @NudurupatiSurya, thank you for your interest, we'd be happy to accept your contributions!

  • Please ensure you sign the Google CLA before beginning
  • You can find developer docs at https://github.com/google/ground-platform/wiki/Ground-Developer's-Guide. They'll help you get set up to test end-to-end
  • Feel free to send a draft PR with a rough sketch of what you plan to add/change so we can make sure we're on the same page.
  • Do you have an ETA for a draft PR? If the work can be timeboxed happy to assign to you.

Thanks again!

@NudurupatiSurya
Copy link
Contributor

Hello, @gino-m.

That's great! I signed the Google CLA yesterday night using the link you provided, and with the help of the documentation, I was able to successfully build the ground Android application.

However, there is an issue with signing in to the app. Because I do not have the profile-refresh cloud function in my Google Cloud, the app is not allowing me to sign in because it is trying to execute that function in the cloud with each sign-in.

Do you know a way around this? I was thinking maybe we can use the result of the profile-refresh function when it is successful but do Let me know your thoughts.

I am also a full-time master’s student at Cal State East Bay so, I should have a draft PR ready within 2 days of resolving this issue.

Thank you! I look forward to contributing to Google Ground!
Surya

@gino-m
Copy link
Collaborator Author

gino-m commented Mar 25, 2024

Glad you were able to get up and running! Have you tried starting the local Firebase emulator as per the docs, and running the Android app on the Android emulator with build variant localDebug? The profile-refresh function should work in the emulator as well. Let me know!

@NudurupatiSurya
Copy link
Contributor

NudurupatiSurya commented Mar 28, 2024

Hello @gino-m, just wanted to give you an update,

I have missed the point of using the Firebase emulator and changing the build variant to localDebug. I have tried it now with the Firebase emulator but I am facing the issue below when I want to sign in. (PFA error log)

Screenshot 2024-03-27 at 10 10 14 PM

To resolve this issue I have tried multiple things, invalidating the cache and restarting the Android studio, verifying the SHA 1 in the Firebase console and all other settings, checking the device’s network connection, and also trying with a different network.

Have you or your team faced anything like this before? If so please share how to resolve this.

@gino-m
Copy link
Collaborator Author

gino-m commented Mar 28, 2024

@NudurupatiSurya thanks for the update! Did you first start the local Firebase emulator as per https://github.com/google/ground-platform/wiki/Ground-Developer's-Guide#run-locally-optional ? Can you see the emulator suite UI at http://localhost:4000/ ?

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

I haven't started the local Firebase emulator first. I did it now and it is working for me—thank you for that guidance.

As I move forward with the draft PR, could you please provide some clarification on a couple of points regarding the sign-out feature:

• The app currently has a sign-out option in the navigation drawer, which triggers a pop-up warning about unsynced data upon attempting to sign out. For this task, are we looking to introduce an additional pop-up that displays the user's name and email ID (as shown in the screenshot you have shared) before showing the unsynced data warning, or should the new pop-up replace the existing unsynced data alert?

Looking forward to your input to ensure I’m moving in the right direction.

Thanks again,
Surya

@gino-m
Copy link
Collaborator Author

gino-m commented Mar 29, 2024

I did it now and it is working for me—thank you for that guidance.

Glad it's working now!

• The app currently has a sign-out option in the navigation drawer, which triggers a pop-up warning about unsynced data upon attempting to sign out. For this task, are we looking to introduce an additional pop-up that displays the user's name and email ID (as shown in the screenshot you have shared) before showing the unsynced data warning, or should the new pop-up replace the existing unsynced data alert?

Good question. The warning would appear only once the user taps "Sign out", so it would be in addition to the new "profile popup" shown above. Note that there's a small typo in the original design, the button should read "Sign out", and not "Signout".

Excited you're taking this on!

@NudurupatiSurya
Copy link
Contributor

Got it, that's clear now. Thanks so much for your guidance!

I'm really excited to contribute to Google Ground as well! 🤩

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

I've created a draft PR with the changes here: PR #2422. Could you please review it and let me know if there are any additions needed or if anything should be adjusted? Looking forward to your guidance to ensure everything is on track.

Thanks!

@gino-m
Copy link
Collaborator Author

gino-m commented Apr 1, 2024

Hi @NudurupatiSurya, thanks for sharing! I think there might be some confusion about what this issue is asking for. I've added some clarifying text to this issue, please let me know if that's clearer!

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m, It’s much clearer now with the added clarifications—thank you for taking the time to do that. I have reviewed the updated details and will change the code accordingly. If I have any further questions or need additional guidance as I proceed, I’ll reach out to ensure everything is aligned with the project’s goals.

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

I've created a draft PR with updated code. Click Here. Could you please review it and let me know if there are any adjustments needed? I look forward to your feedback! 😊

Thank you!

@NudurupatiSurya
Copy link
Contributor

NudurupatiSurya commented Apr 8, 2024

Hello @gino-m,

Thank you for your feedback. I'm currently working on the changes and aim to update the PR by tomorrow. I'll let you know once it's done 😊

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

Thank you for your feedback on the PR. I have implemented the requested changes and merged my local branch with the master branch of my repository as you suggested. I've now opened a new pull request PR #2434.

Could you please review it and let me know if any further modifications are needed? I look forward to your feedback 😊

Thank you!

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m,

Thank you for your code review. I have resolved the conflicts as discussed and also addressed feedback from @shobhitagarwal1612.

The PR is now marked as ready for review. Do let me know if there are any further modifications needed. Looking forward to my first merge! 😃

Thank you!

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m & @shobhitagarwal1612,

Thank you for the feedback! I will make the changes and will update you.

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m & @shobhitagarwal1612,

Just wanted to let you know that I'm currently caught up with my university work this week. I plan to update the PR with your suggestions and implement the new navigation drawer header by next week.

Best regards,
Surya

@NudurupatiSurya
Copy link
Contributor

Hello @gino-m and @shobhitagarwal1612,

Thank you for your guidance in resolving this issue.

If there's any other issue where you need help, please let me know. Otherwise, I will select another issue from the list to work on.

Looking forward to your direction.

Best regards,
Surya

@shobhitagarwal1612
Copy link
Member

Thanks for working on this issue. Let us know if you need any help picking another issue.

@gino-m gino-m closed this as completed May 28, 2024
@gino-m gino-m reopened this May 28, 2024
gino-m pushed a commit that referenced this issue May 28, 2024
)

* Nudurupati surya 2 2397 (#1)

* Initial Commit

* Working - 1

* Small change

* Updated Code based on the requirements

* Removed Signout from NavDrawer

* Modified code to retrieve user details using authentication manager

* Reverted the deletion of google-services.json

* added few imports

* Modified code to adhere to SRP

* added google-services.json

* some refactoring

* Made suggested changes

* Rearranged the order of buttons in the dialog.

* Modified Nav Header

* Modified font family and marginEnd for user photo in navigation drawer

* Changed font to SP & marginEnd for user photo to DP

* Made requested changes

* Fixed Lint Errors

---------

Co-authored-by: Shobhit Agarwal <ashobhit@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue type: fr Request for new feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants