Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list #4340

vinayak0505 · 2021-04-08T14:04:21Z

Description (required)
I am sorry for previous pull request as i panicked and push the wrong one

What changes did you make and why?
I just added a feature to check it the clicked item is loaded before setting that item, which will remove image flicker as program will not wait for 5 milliseconds and random crashes

Tests performed (required)
manual testing

please check this i am really sorry about previous pull request @nicolas-raoul @madhurgupta10 @neslihanturan

codecov-io · 2021-04-08T14:16:02Z

Codecov Report

Merging #4340 (1401c6d) into master (58408cf) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##             master    #4340   +/-   ##
=========================================
  Coverage     10.22%   10.22%           
  Complexity      469      469           
=========================================
  Files           342      342           
  Lines         13084    13084           
  Branches       1062     1063    +1     
=========================================
  Hits           1338     1338           
  Misses        11678    11678           
  Partials         68       68

Impacted Files	Coverage Δ	Complexity Δ
...e/nrw/commons/explore/ExploreListRootFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ee/nrw/commons/media/MediaDetailPagerFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`

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 58408cf...1401c6d. Read the comment docs.

nicolas-raoul · 2021-04-08T14:18:25Z

No problem!
By the way, you can edit a pull request simply by pushing to the same remote branch. No need to close the PR and open a new one :-)

vinayak0505 · 2021-04-08T14:21:48Z

Thank you , I panicked a lot so I thought of starting from scratch , and also know if you like we can add animation, maybe similar to the one when you press notification bell

…

On Thu, 8 Apr, 2021, 7:48 pm Nicolas Raoul, ***@***.***> wrote: No problem! By the way, you can edit a pull request simply by pushing to the same remote branch. No need to close the PR and open a new one :-) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANMXXJ4O7VXN6L6HMARMIELTHW3MPANCNFSM42S63HJQ> .

nicolas-raoul

It works great!
Tapping a contribution now leads to a very smooth experience (I mean there is no flickering).

I just asked for a few minor changes.
Thanks!

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java

vinayak0505 · 2021-04-08T15:35:57Z

done

neslihanturan · 2021-04-08T16:34:53Z

When I was logged out and tried to display a media detail, it crashed with:

free.nrw.commons, PID: 29828
    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)
2021-04-08 19:33:20.862 29828-4650/fr.free.nrw.commons E/ACRA: ACRA caught a NullPointerException for fr.free.nrw.commons
    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)

neslihanturan · 2021-04-08T16:37:20Z

I works good on contributions list, however it crashes then it comes to explore fragment:

    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)
2021-04-08 19:36:22.588 29830-5120/fr.free.nrw.commons E/ACRA: ACRA caught a NullPointerException for fr.free.nrw.commons
    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)
        ```

vinayak0505 · 2021-04-08T16:47:01Z

When I was logged out and tried to display a media detail, it crashed with:

free.nrw.commons, PID: 29828
    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)
2021-04-08 19:33:20.862 29828-4650/fr.free.nrw.commons E/ACRA: ACRA caught a NullPointerException for fr.free.nrw.commons
    java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.media.MediaDetailPagerFragment$MediaDetailAdapter.getCount()' on a null object reference
        at fr.free.nrw.commons.media.MediaDetailPagerFragment$1.run(MediaDetailPagerFragment.java:354)
        at java.lang.Thread.run(Thread.java:919)

I see the adapter is not initialize here

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java

vinayak0505 · 2021-04-08T16:53:41Z

nice catch, as you can see here adapter is not initialized for first 100 millisecond which cases a problem so there are two ways to solve this if i wait for adapter to get initialize or the right way ill fix this adapter, ill try to go with 2nd option first but i have no idea why the programmer has done this and why this was allowed

neslihanturan · 2021-04-08T16:59:34Z

And welcome to the project @vinayak0505 , please do not get stressed if something goes wrong with the PR. It is always normal to close them to start from scratch. It is normal to send new commits to the existing PR too.

neslihanturan · 2021-04-08T17:04:12Z

nice catch, as you can see here adapter is not initialized for first 100 millisecond which cases a problem so there are two ways to solve this if i wait for adapter to get initialize or the right way ill fix this adapter, ill try to go with 2nd option first but i have no idea why the programmer has done this and why this was allowed

I am not really sure why this delay is there, but considering this is a network operation, I assume it might be required. Anyway please share your further findings with us :)

vinayak0505 · 2021-04-08T17:07:32Z

And welcome to the project @vinayak0505 , please do not get stressed if something goes wrong with the PR. It is always normal to close them to start from scratch. It is normal to send new commits to the existing PR too.

Thank you very much and about stressing that you just mentioned if it's because you read the above comments then thank you I felt really good but if something I might blame a mistake on others for my PR then don't worry If I have do a mistake I'll accept that even if I lose an opportunity

vinayak0505 · 2021-04-08T17:31:04Z

hey!! thanks for the report but please create a different bug report for this as i have figured out what was wrong as problem was in the explore Fragment code i can solve this hear but then i wont get point for GSoC, if creating a issue would problematic then no issue ill upload it here only, ill make point from other bugs no issue @neslihanturan

neslihanturan · 2021-04-08T17:49:52Z

hey!! thanks for the report but please create a different bug report for this as i have figured out what was wrong as problem was in the explore Fragment code i can solve this hear but then i wont get point for GSoC, if creating a issue would problematic then no issue ill upload it here only, ill make point from other bugs no issue @neslihanturan

We don't only consider number of PRs, we consider their difficulty level for GSoC. There is not a second issue here, this issue covers all of the steps we talked about. So there is no need to create one more issue or one more PR.

Can you please commit your new solution (it is hard to follow from code screen shoots, I prefer to see commits instead)?

Also, sometimes I am having hard times to understand your messages. Could you please make sure you follow punctuation rules? So that I could follow sentence ends and beginnings. Thanks!:)

vinayak0505 · 2021-04-08T17:53:04Z

I am working on solving this.

I'll take care of punctuation from next time.

vinayak0505 · 2021-04-08T18:47:38Z

solved please review it.

nicolas-raoul

Thanks for fixing the stylistic issues!

I just tested with this branch's latest code, it works great for me even when scrolling and tapping like crazy.

Neslihan, if you know a way to trigger the crash with higher probability (for instance by tapping fast in a particular order), please let us know :-)

vinayak0505 · 2021-04-09T04:08:26Z

Can I start working on other issues?
I have checked from my side as this function is used in five different places and I have checked them

nicolas-raoul · 2021-04-09T04:54:27Z

Yes, feel free to work on other issues while we test this branch further. By the way, I suggest you create a new branch for each pull request, rather than working directly on your master branch.

…

On Fri, 9 Apr 2021, 13:08 Vinayak Aggarwal, ***@***.***> wrote: Can I start working on other issues? I have checked from my side as this function is used in five different places and I have checked them — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAYKBWAQDBRFRQFCI4FTVDTHZ4UTANCNFSM42S63HJQ> .

vinayak0505 · 2021-04-10T03:56:49Z

@neslihanturan please review this :)

nicolas-raoul

I have been using this branch for 2 days, it is working great so far, I really appreciate that flickering (previous image shown briefly upon tapping) does not happen anymore.

Apart from the small detail I noticed right now, the code looks reasonable to me.

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java

vinayak0505 · 2021-04-11T13:55:06Z

@neslihanturan I have made changes as per your requirement. so please check.

…commons

…android-commons" This reverts commit d215a2e, reversing changes made to 27754f3.

vinayak0505 · 2021-04-11T15:11:44Z

I have merged upstream also please check

neslihanturan · 2021-04-12T14:44:10Z

Thanks @vinayak0505 and @nicolas-raoul , I am on it :)

This reverts commit fba7fb8.

neslihanturan

Hey @vinayak0505 there are several irrelevant changes made in this PR, please revert them. Only required lines to solve the issue should be changed. So when you switched to files tab on github (or when you check git diff) you should only see related lines are edited. Please make sure you create a new branches on each task, otherwise (if you work on master for all tasks) such problems may occur. You can follow our contributors guideline for information about git flows. Please ping me for review as soon as you solved this issue.

If you are confused feel free to close this PR and open a new one.

vinayak0505 · 2021-04-13T02:48:59Z

@neslihanturan in last 2 commits I only pull upstream to check , I revert them back but they didn't update. can you merge with leaving the last 2 commits

vinayak0505 · 2021-04-13T02:49:40Z

Check the third last commit

nicolas-raoul · 2021-04-13T02:56:03Z

@vinayak0505 It is a good thing to keep PRs up-to-date, your intent was great.
To do so, better use Git's rebase operation, though. Would you mind trying?
If it does not work, an easy workaround is to create a new PR as Neslihan suggested.
Thanks!

nicolas-raoul · 2021-04-13T03:05:14Z

I would be great if you could do it for this one :-)

…pp/apps-android-commons"" This reverts commit d7e06de.

…android-commons" This reverts commit d215a2e, reversing changes made to 27754f3.

…pp/apps-android-commons"" This reverts commit d7e06de.

…commons

vinayak0505 · 2021-04-13T04:28:00Z

@nicolas-raoul I revert back the changes and made a rebase but then it shows more files changed so i created a new pull request

vinayak0505 · 2021-04-13T08:32:43Z

@neslihanturan please check pull request #4354

Fixes #3879 #4283

d0a72d2

nicolas-raoul requested changes Apr 8, 2021

View reviewed changes

Update MediaDetailPagerFragment.java

5397508

neslihanturan changed the title ~~Fixes #3879 #4283~~ Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list Apr 8, 2021

neslihanturan requested changes Apr 8, 2021

View reviewed changes

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java Outdated Show resolved Hide resolved

vinayak0505 added 4 commits April 8, 2021 23:26

Done

954dd84

Update MediaDetailPagerFragment.java

59f9da8

Update MediaDetailPagerFragment.java

67b8f79

Update MediaDetailPagerFragment.java

742804d

vinayak0505 requested a review from neslihanturan April 8, 2021 18:47

nicolas-raoul reviewed Apr 9, 2021

View reviewed changes

nicolas-raoul requested changes Apr 10, 2021

View reviewed changes

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java Outdated Show resolved Hide resolved

nicolas-raoul approved these changes Apr 11, 2021

View reviewed changes

vinayak0505 added 2 commits April 11, 2021 20:15

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

d215a2e

…commons

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

d7e06de

…android-commons" This reverts commit d215a2e, reversing changes made to 27754f3.

translatewiki and others added 2 commits April 12, 2021 15:40

Localisation updates from https://translatewiki.net.

504408e

Add Unit Tests for Upload Service (#4322)

fba7fb8

Revert "Add Unit Tests for Upload Service (#4322)" (#4353)

4bca142

This reverts commit fba7fb8.

neslihanturan requested changes Apr 12, 2021

View reviewed changes

Revert "Revert "Merge branch 'master' of https://github.com/commons-a…

1401c6d

…pp/apps-android-commons"" This reverts commit d7e06de.

vinayak0505 closed this Apr 13, 2021

vinayak0505 reopened this Apr 13, 2021

vinayak0505 added 10 commits April 13, 2021 08:58

Fixes #3879 #4283

a4a31a4

Update MediaDetailPagerFragment.java

c7eca8f

Done

0a453aa

Update MediaDetailPagerFragment.java

d30a2e4

Update MediaDetailPagerFragment.java

0d4c0cb

Update MediaDetailPagerFragment.java

677ddb8

Update MediaDetailPagerFragment.java

7236e4e

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

e38e761

…android-commons" This reverts commit d215a2e, reversing changes made to 27754f3.

Revert "Revert "Merge branch 'master' of https://github.com/commons-a…

c7dcc4a

…pp/apps-android-commons"" This reverts commit d7e06de.

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

9e1a715

…commons

vinayak0505 closed this Apr 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list #4340

Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list #4340

vinayak0505 commented Apr 8, 2021

codecov-io commented Apr 8, 2021 •

edited

nicolas-raoul commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021 via email

nicolas-raoul left a comment

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

neslihanturan commented Apr 8, 2021 •

edited

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

nicolas-raoul left a comment

vinayak0505 commented Apr 9, 2021

nicolas-raoul commented Apr 9, 2021 via email

vinayak0505 commented Apr 10, 2021

nicolas-raoul left a comment

vinayak0505 commented Apr 11, 2021

vinayak0505 commented Apr 11, 2021 •

edited

neslihanturan commented Apr 12, 2021

neslihanturan left a comment

vinayak0505 commented Apr 13, 2021 •

edited

vinayak0505 commented Apr 13, 2021

nicolas-raoul commented Apr 13, 2021

nicolas-raoul commented Apr 13, 2021

vinayak0505 commented Apr 13, 2021

vinayak0505 commented Apr 13, 2021

Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list #4340

Fixes #3879 #4283 Other pictures flickers when a media is selected from contributions list #4340

Conversation

vinayak0505 commented Apr 8, 2021

codecov-io commented Apr 8, 2021 • edited

Codecov Report

nicolas-raoul commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021 via email

nicolas-raoul left a comment

Choose a reason for hiding this comment

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

neslihanturan commented Apr 8, 2021 • edited

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

neslihanturan commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

vinayak0505 commented Apr 8, 2021

nicolas-raoul left a comment

Choose a reason for hiding this comment

vinayak0505 commented Apr 9, 2021

nicolas-raoul commented Apr 9, 2021 via email

vinayak0505 commented Apr 10, 2021

nicolas-raoul left a comment

Choose a reason for hiding this comment

vinayak0505 commented Apr 11, 2021

vinayak0505 commented Apr 11, 2021 • edited

neslihanturan commented Apr 12, 2021

neslihanturan left a comment

Choose a reason for hiding this comment

vinayak0505 commented Apr 13, 2021 • edited

vinayak0505 commented Apr 13, 2021

nicolas-raoul commented Apr 13, 2021

nicolas-raoul commented Apr 13, 2021

vinayak0505 commented Apr 13, 2021

vinayak0505 commented Apr 13, 2021

codecov-io commented Apr 8, 2021 •

edited

neslihanturan commented Apr 8, 2021 •

edited

vinayak0505 commented Apr 11, 2021 •

edited

vinayak0505 commented Apr 13, 2021 •

edited