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

Resolves Reloading of image on moving to next question in peer review #2709

Merged
merged 4 commits into from
Mar 23, 2019

Conversation

vanshikaarora
Copy link
Collaborator

Description (required)

Fixes sub-task of #2698 to stop reloading of image while moving to the next question for the same image.
Fixes #2698 Improve peer review feature quality

Changes made

  • Removed the image and caption from ReviewImageFragment.java to ReviewActivity.java
  • Correspondingly loading image and progressbar in ReviewActivity.java

@codecov-io
Copy link

codecov-io commented Mar 22, 2019

Codecov Report

Merging #2709 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2709      +/-   ##
=========================================
+ Coverage    2.72%   2.72%   +<.01%     
=========================================
  Files         267     267              
  Lines       12814   12801      -13     
  Branches     1140    1137       -3     
=========================================
  Hits          349     349              
+ Misses      12439   12426      -13     
  Partials       26      26
Impacted Files Coverage Δ
...r/free/nrw/commons/review/ReviewImageFragment.java 0% <0%> (ø) ⬆️
...ava/fr/free/nrw/commons/review/ReviewActivity.java 0% <0%> (ø) ⬆️
...fr/free/nrw/commons/review/ReviewPagerAdapter.java 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 239f749...7b06c16. Read the comment docs.

@domdomegg domdomegg self-requested a review March 23, 2019 14:59
@domdomegg
Copy link
Member

Please can you resolve the merge conflicts and I can review.

@domdomegg domdomegg removed their request for review March 23, 2019 15:14
@vanshikaarora
Copy link
Collaborator Author

@domdomegg I have resolved the merge conflicts. Kindly review :)

@domdomegg domdomegg self-requested a review March 23, 2019 16:53
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes:

  • Skip image text layout
  • Missing padding above question
  • Author name bug
master 2709
Screenshot_1553361248 Screenshot_1553361174

@vanshikaarora
Copy link
Collaborator Author

@domdomegg I have made the changes. Here is the modified UI.
Kindly review :)

WhatsApp Image 2019-03-23 at 11 08 24 PM

@domdomegg domdomegg self-requested a review March 23, 2019 21:39
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested 2.10.1-debug-peer_viewpager~7c64ff775

The "SKIP THIS IMAGE" text still looks a bit weird for me. However, I'm going to merge this as it's quite a large change and don't want to block others working on peer review with merge conflicts. Also the layout needs to be fixed in general so it's a bit pointless fiddling with minor layout things here.

Screenshot_1553377737

@domdomegg domdomegg merged commit cbab7dd into commons-app:master Mar 23, 2019
@vanshikaarora
Copy link
Collaborator Author

Thanks for approving the PR @domdomegg :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants