Fix #777 : Add Profile Picture Edit Dialog #801

aggarwalpulkit596 · 2020-03-14T06:14:51Z

Explanation

Fixes #777

Shows a pop-up dialog for Changing/Viewing Profile Picture

Mock Link: https://xd.adobe.com/view/e8aa4198-3940-47f9-514a-f41cc54457f6-9e9b/screen/7fcddeef-4d62-4565-b763-81e89f064a5d/UP-O-Forgot-PIN-Dialog-

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

aggarwalpulkit596 · 2020-03-14T06:15:18Z

@rt4914 Can you take a look whether the approach is correct or not

rt4914

The approach looks great.

Also, I know that you were just looking for suggestion but I still did a full review so that you fix those things in next PR along with full implementation and test cases. Thanks.

app/src/main/java/org/oppia/app/profileprogress/ProfilePictureDialogInterface.kt

app/src/main/java/org/oppia/app/profileprogress/ProfilePictureEditDialogFragment.kt

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressFragment.kt

app/src/main/res/values/strings.xml

aggarwalpulkit596 · 2020-03-15T03:53:20Z

How do i set the presenter for profileprogressheader binding adapter i couldn't any method through which i can achieve this

rt4914 · 2020-03-15T08:39:50Z

How do i set the presenter for profileprogressheader binding adapter i couldn't any method through which i can achieve this

@aggarwalpulkit596 I am actually not able to understand what you mean here.

aggarwalpulkit596 · 2020-03-15T08:43:35Z

@rt4914 the thing is that right now my onclick functions are not invoking and that's because i've haven't set the presenter to BindingAdapter of RecyclerView

rt4914 · 2020-03-15T08:49:27Z

@rt4914 the thing is that right now my onclick functions are not invoking and that's because i've haven't set the presenter to BindingAdapter of RecyclerView

Actually your viewmodel should be responsible for handling the clicks instead of presenter.

Checkout OngoingStoryViewModel and see how the item click function is getting used in xml file. At the same time, after click that function is handling everything using an interface.

aggarwalpulkit596 · 2020-03-16T04:51:23Z

@rt4914 i've pushed some more changes can you please take a look at the implementation

rt4914

@aggarwalpulkit596 Great. Now this is perfect implementation. Thanks.

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressActivity.kt

app/src/main/java/org/oppia/app/profileprogress/ProfilePictureClickListener.kt

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressFragmentPresenter.kt

aggarwalpulkit596 · 2020-03-16T05:08:23Z

@rt4914 and for the test do I need to create a separate file for ProfileProgressActivity because there are no test written for that activity

rt4914 · 2020-03-16T05:10:20Z

@rt4914 and for the test do I need to create a separate file for ProfileProgressActivity because there are no test written for that activity

You can use ProfileProgressFragmentTest but also, please implement those two functionalities also, what will happen when we select one of the options in the pop-up. We need full functionality in this PR.

aggarwalpulkit596 · 2020-03-16T05:13:51Z

@rt4914 do i need to open the gallery intent as well ?

rt4914 · 2020-03-16T05:23:23Z

@rt4914 do i need to open the gallery intent as well ?

Yes, but I believe it should be present in one of the files already. Check ProfileEditActivity or AddProfileActivity.

aggarwalpulkit596 · 2020-03-16T05:29:12Z

@rt4914 i've added one test and added intent to open Gallery do i need to do anything else

rt4914 · 2020-03-16T05:54:45Z

@rt4914 i've added one test and added intent to open Gallery do i need to do anything else

Please finish the PR implementation in which ever way you think is correct because going back and forth on a PR on every step is not possible. Please finish the PR completely from your end and then assign it back to me.

aggarwalpulkit596 · 2020-03-16T08:11:14Z

@rt4914 should I create an activity as well to display the profile picture in full screen?

aggarwalpulkit596 · 2020-03-16T08:19:05Z

@rt4914 also there is no option to update the profile picture inside ProfileManagementController

aggarwalpulkit596 · 2020-03-16T12:18:29Z

@rt4914 also there is no option to update the profile picture inside ProfileManagementController

I will create a PR for that.

Done. Merge your branch with latest develop. Also, i have not tested that function in app module so it you face difficulties, please do let me know.

@rt4914 I've checked avatar change part works fine
Thanks

aggarwalpulkit596 · 2020-03-16T13:23:33Z

@rt4914 completed the PR
thank you for assigning this issue a lot of things are now crystal clear about the project

rt4914

Overall good great. Suggested some changes.

app/src/main/AndroidManifest.xml

app/src/main/java/org/oppia/app/profileprogress/ProfilePictureActivity.kt

app/src/main/res/layout/profile_picture_activity.xml

app/src/sharedTest/java/org/oppia/app/profileprogress/ProfileProgressFragmentTest.kt

aggarwalpulkit596 · 2020-03-16T16:39:47Z

@rt4914 i guess this should be the final review

rt4914

Suggested few nit changes

app/src/main/java/org/oppia/app/profileprogress/ProfilePictureActivityViewModel.kt

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressActivityPresenter.kt

app/src/sharedTest/java/org/oppia/app/profileprogress/ProfilePictureActivityTest.kt

rt4914

Nit Change continuing from previous review.

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressActivityPresenter.kt

rt4914

This looks good. Thanks a lot.

Add Profile Picture Edit Dialog

e171ed8

aggarwalpulkit596 requested a review from rt4914 March 14, 2020 06:15

aggarwalpulkit596 assigned rt4914 Mar 14, 2020

rt4914 reviewed Mar 14, 2020

View reviewed changes

rt4914 assigned aggarwalpulkit596 and unassigned rt4914 Mar 14, 2020

dialog opening correctly

6ce4220

Add ProfilePictureClickListener

cfeb08c

rt4914 reviewed Mar 16, 2020

View reviewed changes

Handle ProfilePictureEditDialog Click Actions

1bc806f

Add test

260bd80

aggarwalpulkit596 requested a review from rt4914 March 16, 2020 05:28

aggarwalpulkit596 assigned rt4914 Mar 16, 2020

rt4914 removed their assignment Mar 16, 2020

Handle Gallery Photo Selection

6a83a7d

aggarwalpulkit596 removed their assignment Mar 16, 2020

Add ProfilePictureActivity

02102f9

aggarwalpulkit596 assigned rt4914 and unassigned aggarwalpulkit596 Mar 16, 2020

rt4914 suggested changes Mar 16, 2020

View reviewed changes

rt4914 assigned aggarwalpulkit596 and unassigned rt4914 Mar 16, 2020

aggarwalpulkit596 added 2 commits March 16, 2020 22:03

nit changes

e0947e5

r

60ca2fb

aggarwalpulkit596 requested a review from rt4914 March 16, 2020 16:56

aggarwalpulkit596 assigned rt4914 and unassigned aggarwalpulkit596 Mar 16, 2020

rt4914 suggested changes Mar 17, 2020

View reviewed changes

rt4914 assigned aggarwalpulkit596 and unassigned rt4914 Mar 17, 2020

final nit changes

6626ebb

aggarwalpulkit596 requested a review from rt4914 March 17, 2020 08:46

aggarwalpulkit596 assigned rt4914 and unassigned aggarwalpulkit596 Mar 17, 2020

rt4914 suggested changes Mar 17, 2020

View reviewed changes

app/src/main/java/org/oppia/app/profileprogress/ProfileProgressActivityPresenter.kt Outdated Show resolved Hide resolved

rt4914 assigned aggarwalpulkit596 and unassigned rt4914 Mar 17, 2020

some nit changes

9ec53cc

aggarwalpulkit596 requested a review from rt4914 March 17, 2020 08:51

aggarwalpulkit596 assigned rt4914 and unassigned aggarwalpulkit596 Mar 17, 2020

rt4914 approved these changes Mar 17, 2020

View reviewed changes

rt4914 merged commit f415961 into oppia:develop Mar 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #777 : Add Profile Picture Edit Dialog #801

Fix #777 : Add Profile Picture Edit Dialog #801

aggarwalpulkit596 commented Mar 14, 2020

aggarwalpulkit596 commented Mar 14, 2020

rt4914 left a comment

aggarwalpulkit596 commented Mar 15, 2020

rt4914 commented Mar 15, 2020

aggarwalpulkit596 commented Mar 15, 2020

rt4914 commented Mar 15, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

rt4914 left a comment

rt4914 left a comment

Fix #777 : Add Profile Picture Edit Dialog #801

Fix #777 : Add Profile Picture Edit Dialog #801

Conversation

aggarwalpulkit596 commented Mar 14, 2020

Explanation

Checklist

aggarwalpulkit596 commented Mar 14, 2020

rt4914 left a comment

Choose a reason for hiding this comment

aggarwalpulkit596 commented Mar 15, 2020

rt4914 commented Mar 15, 2020

aggarwalpulkit596 commented Mar 15, 2020

rt4914 commented Mar 15, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

Choose a reason for hiding this comment

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

Choose a reason for hiding this comment

aggarwalpulkit596 commented Mar 16, 2020

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment