#4730 - Nothing happens on clicking media in the Contribution tab of User's Profile#4736
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4736 +/- ##
============================================
+ Coverage 40.01% 40.24% +0.22%
- Complexity 1760 1802 +42
============================================
Files 358 363 +5
Lines 15295 15659 +364
Branches 1320 1358 +38
============================================
+ Hits 6121 6302 +181
- Misses 8725 8889 +164
- Partials 449 468 +19
Continue to review full report at Codecov.
|
4D17Y4
left a comment
There was a problem hiding this comment.
Looks good to me, works well.
Just some documentation on the added function, variables and conditional statements.
Also, I think it would be better to have the top bar with bookmark, share, download options as in contribution and explore. @nicolas-raoul views ?
neslihanturan
left a comment
There was a problem hiding this comment.
Other than my review, the code looks nice. Thanks a lot @Pratham2305 :)
| super.onCreate(savedInstanceState); | ||
| if (getArguments() != null) { | ||
| userName = getArguments().getString(KEY_USERNAME); | ||
| isUserProfile = true; |
There was a problem hiding this comment.
If we create ContributionsFragment with arguments in some other pieces of the code than ProfileActivity, the bundle may still be not null and this code will set isUserProfile boolean to true. However, this may not necessarily be the case. So, I think this boolean should be set when KEY_USERNAME argument is not null rather than when any kind of argument is not null.
|
|
||
| @Override | ||
| public void onBackPressed() { | ||
| if(contributionsFragment != null && contributionsFragment.getMediaDetailPagerFragment() != null && contributionsFragment.getMediaDetailPagerFragment().isVisible()) { |
There was a problem hiding this comment.
Maybe adding a comment to explain this if condition?
| } | ||
| } | ||
|
|
||
| if (getActivity() instanceof ProfileActivity) { |
There was a problem hiding this comment.
adding some comment to explain this iftoo would be nice
| ((BaseActivity)getActivity()).getSupportActionBar().setDisplayHomeAsUpEnabled(false); | ||
| ((MainActivity)getActivity()).showTabs(); | ||
| fetchCampaigns(); | ||
| //((BaseActivity)getActivity()).getSupportActionBar().setDisplayHomeAsUpEnabled(false); |
There was a problem hiding this comment.
Removing this line and explaining the lines at the bottom would be cool
neslihanturan
left a comment
There was a problem hiding this comment.
Some comments should be added and style issues should be fixed
|
|
||
| setNotificationCount(); | ||
| fetchCampaigns(); | ||
| if(!isUserProfile) { |
There was a problem hiding this comment.
The if needs explanation comment
neslihanturan
left a comment
There was a problem hiding this comment.
Thanks a lot @Pratham2305 :)
Description (required)
Fixes #4730
What changes did you make and why?
Added ContributionFragment to the ViewPager instead of ContributionListFragment in Profile Activity
Used the ContributionFragment similar to the way it used in Contribution Tab, to handle MediaDetailPagerFragment and ContributionListFragment transition.
Disable unnecessary ContributionFragment features when it is used in ProfileActivity, like campaigns, nearby notification, etc.
Added onBackPressed() method in ProfileActivity, and also replaced viewPager with ParentViewPager.
Tests performed (required)
Tested ProdDebug on Pixel 3 with API level 29.