#4005 For images in set removed if user selected one image #4022

gouri-panda · 2020-11-07T13:18:55Z

What changes did you make and why?
Created a variable for storing if the user selected multiple images or not in UploadActivity then it will access by the fragment
and then it will set the text depends on the variable.

gouri-panda · 2020-11-07T13:20:10Z

Ping @nicolas-raoul for review.

nicolas-raoul

Looks good, only minor changes to perform.

nicolas-raoul · 2020-11-07T14:56:27Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

    private Place place;
    private List<UploadableFile> uploadableFiles = Collections.emptyList();
    private int currentSelectedPosition = 0;
+    private boolean isMultipleItemSelected = false;


"Item" has a different meaning in our app. Please rename to isMultipleFilesSelected thanks!

nicolas-raoul · 2020-11-07T14:58:33Z

app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java

                .subscribe(filter -> searchForCategory(filter.toString()), Timber::e);
    }
+    private void setTvSubTitle() {
+        Log.d("gouri", "setTvSubTitle: in uploadfragments");


Not needed I guess? 🙂

D'oh! sorry I was debugging something.

gouri-panda · 2020-11-13T12:29:36Z

@nicolas-raoul Is there something wrong with PR?

nicolas-raoul · 2020-11-15T01:26:26Z

@gouri-panda It looks great but I have been super busy and almost no time for this app reecently :'-(

Could anyone test this PR? Thanks a lot!

gouri-panda · 2020-11-15T04:20:28Z

@nicolas-raoul No rush take your time.

nicolas-raoul · 2021-02-23T09:07:03Z

@gouri-panda Very sorry for the delay, I now have time to review your PRs :-)
The change looks good, but would you mind rebasing on the latest master? Thanks!

gouri-panda · 2021-02-23T09:08:50Z

@nicolas-raoul sure!

…commons into pull/4022 and resolved conflicts

gouri-panda · 2021-02-23T14:08:06Z

@nicolas-raoul @neslihanturan This PR is ready now.

nicolas-raoul · 2021-02-23T14:20:09Z

I tried compiling but could not: MediaLicenseFragment.java:81: error: ';' expected

nicolas-raoul

Probably need to fix syntax?

codecov-io · 2021-02-23T14:56:16Z

Codecov Report

Merging #4022 (355fe0a) into master (0ec6217) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4022      +/-   ##
============================================
- Coverage     10.41%   10.37%   -0.04%     
  Complexity      474      474              
============================================
  Files           343      342       -1     
  Lines         13063    13118      +55     
  Branches       1051     1067      +16     
============================================
+ Hits           1360     1361       +1     
- Misses        11635    11688      +53     
- Partials         68       69       +1

Impacted Files	Coverage Δ	Complexity Δ
...ava/fr/free/nrw/commons/upload/UploadActivity.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ns/upload/categories/UploadCategoriesFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ee/nrw/commons/upload/depicts/DepictsFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...w/commons/upload/license/MediaLicenseFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...mons/contributions/ContributionBoundaryCallback.kt	`95.45% <0.00%> (-4.55%)`	`6.00% <0.00%> (ø%)`
...earby/presenter/NearbyParentFragmentPresenter.java	`58.52% <0.00%> (-1.02%)`	`36.00% <0.00%> (ø%)`
...ava/fr/free/nrw/commons/utils/PermissionUtils.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...va/fr/free/nrw/commons/campaigns/CampaignView.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...va/fr/free/nrw/commons/nearby/model/ResultTuple.kt	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...va/fr/free/nrw/commons/upload/UploadMediaDetail.kt	`45.45% <0.00%> (ø)`	`3.00% <0.00%> (ø%)`
... and 11 more

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 0ec6217...355fe0a. Read the comment docs.

nicolas-raoul

Tested with both a single picture and multiple pictures, it works as intended.
Thanks a lot!

nicolas-raoul

Sorry a few minor things

nicolas-raoul · 2021-02-25T01:04:24Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

    private Place place;
    private List<UploadableFile> uploadableFiles = Collections.emptyList();
    private int currentSelectedPosition = 0;
+    private boolean isMultipleFilesSelected = false;


Ah, I forgot! We need javadoc for each new method/member.
After that I will merge.

@nicolas-raoul Should we add Javadoc here or in getter fields?

Both methods and members, actually. Sorry for the hassle, but documentation is usually written once, read many times, so it is a good investment :-)

Javadoc must use this format for members too:

/** * Example javadoc */

Thanks!

nicolas-raoul · 2021-02-25T01:04:48Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

        place = intent.getParcelableExtra(PLACE_OBJECT);
        resetDirectPrefs();
    }
+    public boolean getIsMultipleFilesSelected() {


nicolas-raoul · 2021-02-25T01:04:57Z

app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java

                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(filter -> searchForCategory(filter.toString()), Timber::e);
    }
+    private void setTvSubTitle() {


javadoc, and also one empty line before the method, to make it easier to see.

app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java

app/src/main/java/fr/free/nrw/commons/upload/license/MediaLicenseFragment.java

nicolas-raoul · 2021-02-26T08:03:49Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

    private Place place;
    private List<UploadableFile> uploadableFiles = Collections.emptyList();
    private int currentSelectedPosition = 0;
+    private boolean isMultipleFilesSelected = false;


Both methods and members, actually. Sorry for the hassle, but documentation is usually written once, read many times, so it is a good investment :-)

gouri-panda · 2021-02-28T19:05:03Z

@nicolas-raoul Sorry for the delay. You can check now ;)

neslihanturan

Some minor fixes on comments :)

neslihanturan · 2021-03-01T12:59:34Z

app/src/main/java/fr/free/nrw/commons/upload/license/MediaLicenseFragment.java

    }

+    /**
+     * Sets the Tv Subtitle


Thanks @gouri-panda , since the purpose of the last review was adding javadocs could you please be more specific with javadocs instead of just writing the name of the method. Ie. this one could be something like:
"If multiple file is not selected, makes the visibility of subtitle text view gone."

Thanks, @neslihanturan .From now on I'll keep this in my mind.

neslihanturan · 2021-03-01T13:01:30Z

app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java

    }

+    /**
+     * Sets the Depicts subtitle


Same here, could be more definitive. "Sets the Depict subtitle" is not actually what the method does. It decides the visibility of depics text view if multiplefile is not selecred as far as I can understand.

nicolas-raoul · 2021-03-10T05:22:51Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

    private Place place;
    private List<UploadableFile> uploadableFiles = Collections.emptyList();
    private int currentSelectedPosition = 0;
+    private boolean isMultipleFilesSelected = false;


Javadoc must use this format for members too:

/** * Example javadoc */

Thanks!

nicolas-raoul · 2021-03-10T05:36:15Z

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

    private List<UploadableFile> uploadableFiles = Collections.emptyList();
    private int currentSelectedPosition = 0;
-    // Checks for if multiple files selected
+    /*


Needs one more star here and another on the next line :-)
Must look like this:

/** * Example javadoc */

Thanks!

Sorry about that!

neslihanturan · 2021-05-24T08:57:31Z

Hey, this changes looks okay to me to merge, for some reason tests are failing. Do you have any estimation @gouri-panda ?

gouri-panda · 2021-05-24T18:11:47Z

@neslihanturan Thanks for reviewing files but I can't find build logs. Perhaps you can try with a new build tests. Thanks :)

neslihanturan · 2021-08-06T10:18:13Z

I can't see any point in this PR where Travis can fail. I can not see the build either. So merging this one for now to see if the build will fail or not. If so, we can always revert the commit.

commons-app#4005 For images in set removed if user selected one image

6dc0371

nicolas-raoul requested changes Nov 7, 2020

View reviewed changes

commons-app#4005 changed code as suggested

e60ad00

gouri-panda requested a review from nicolas-raoul November 7, 2020 15:18

Merge branch 'master' of https://github.com/commons-app/apps-android-…

4b9e442

…commons into pull/4022 and resolved conflicts

nicolas-raoul requested changes Feb 23, 2021

View reviewed changes

fixed syntax

da4895b

nicolas-raoul approved these changes Feb 25, 2021

View reviewed changes

nicolas-raoul requested changes Feb 25, 2021

View reviewed changes

added javadoc

03a8121

gouri-panda requested a review from nicolas-raoul February 25, 2021 05:40

nicolas-raoul requested changes Feb 26, 2021

View reviewed changes

added more java doc

d23ddb6

neslihanturan requested changes Mar 1, 2021

View reviewed changes

gouri-panda added 2 commits March 8, 2021 23:50

refactored to more meaningful documentation

40f7ade

reverted the Project.xml file

355fe0a

gouri-panda requested review from neslihanturan and nicolas-raoul March 8, 2021 18:25

nicolas-raoul requested changes Mar 10, 2021

View reviewed changes

cleaned documentations a little bit

97dfe30

gouri-panda force-pushed the Remove_For_all_images_set_in_set_from_upload branch from 2589389 to 97dfe30 Compare March 10, 2021 05:40

neslihanturan approved these changes May 24, 2021

View reviewed changes

neslihanturan merged commit d0df2ff into commons-app:master Aug 6, 2021

neslihanturan mentioned this pull request Aug 6, 2021

Remove unnecessary imports and fix type error #4535

Merged

#4005 For images in set removed if user selected one image #4022

#4005 For images in set removed if user selected one image #4022

Uh oh!

Conversation

gouri-panda commented Nov 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gouri-panda commented Nov 7, 2020

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gouri-panda commented Nov 13, 2020

Uh oh!

nicolas-raoul commented Nov 15, 2020

Uh oh!

gouri-panda commented Nov 15, 2020

Uh oh!

nicolas-raoul commented Feb 23, 2021

Uh oh!

gouri-panda commented Feb 23, 2021

Uh oh!

gouri-panda commented Feb 23, 2021

Uh oh!

nicolas-raoul commented Feb 23, 2021

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gouri-panda Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gouri-panda commented Feb 28, 2021

Uh oh!

neslihanturan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gouri-panda commented Nov 7, 2020 •

edited

Loading

codecov-io commented Feb 23, 2021 •

edited

Loading

gouri-panda Feb 25, 2021 •

edited

Loading