Fixed, Hitting back on the DWDS app exits it. #3512
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #3512 +/- ##
=============================================
+ Coverage 48.98% 49.05% +0.07%
- Complexity 1076 1079 +3
=============================================
Files 285 285
Lines 10422 10421 -1
Branches 1396 1395 -1
=============================================
+ Hits 5105 5112 +7
+ Misses 4493 4480 -13
- Partials 824 829 +5 ☔ View full report in Codecov by Sentry. |
| requireActivity().finish() | ||
| return super.onBackPressed(activity) | ||
| } | ||
|
|
||
| private fun openSearchItem(item: SearchItemToOpen) { |
There was a problem hiding this comment.
Why this method is overriden?
There was a problem hiding this comment.
@kelson42 This method was overridden to handle the backPressed event in Custom Apps, it is specially overridden for if the user back presses in custom apps and he is on the Reader screen then we were closing the application.
There was a problem hiding this comment.
talking about opensearchItem() and the question is why it need to be doffetent in a custom app?
There was a problem hiding this comment.
@kelson42 Thanks for highlighting this, We are observing the seachedItem in both modules(app, custom) and this method for opening the searched item, but we have written this method twice in both reader fragments so it would be better to move this code CoreReaderFragment.
There was a problem hiding this comment.
Done.
It does not make sense to me to override a method and just calling "super". Just remove overriding and it should take parent version.
Apart from this change, i have checked in the code there are no other method which is just calling super after overrrding.
MohitMaliFtechiz
force-pushed
the
Issue#3507
branch
from
aa93445 to
25ed008
Compare
|
@MohitMaliDeveloper You still don't have put the code comments as already requested twice on the parent class methods which are overriden for custom app. This is the only solution we have that a future modification on parent class goes applied as well (if necessary) on the custom class. |
|
@kelson42 Sorry I missed adding the comments, now I have added the necessary comments on methods that are override in CustomReaderFragment with the explanation what are the working of those methods. |
|
@MohitMaliFtechiz What I requested on paretn class is atill missing |
I have added the comments in the parent class(CoreReaderFragment). |
It’s not what I was asking for! You still don’t have followed my instructions. Why you don’t just read accurately why I write?! #3507 (comment) #3512 (comment) |
|
@kelson42 Can you please point out which parent class you want those comments or remove the unnecessary code? |
|
@MohitMaliFtechiz Attempt number #5: write a comment on each method of corereaderfragment, wich is superseeded/overriden in the customapp fragment, that way - in the future - the dev is invited to look to the customapp fragment too if it modify this custom app method as well. This will avoid the kind of bug this PR fixes! |
kelson42
force-pushed
the
Issue#3507
branch
2 times, most recently
from
ed8391e to
3391a5d
Compare
|
@MohitMaliFtechiz Please rebase on main (and fix conflicts) |
|
I believe it hasn't yet implemented the request @kelson42 made. I'll come back after it has done. |
MohitMaliFtechiz
force-pushed
the
Issue#3507
branch
2 times, most recently
from
ae93d73 to
51ce43b
Compare
@kelson42 Done.
@gouri-panda I have make the changes can you please review this PR. |
|
@MohitMaliFtechiz Sure! Thanks! |
|
@MohitMaliFtechiz red CI |
CI is green now. |
|
@MohitMaliFtechiz sorry, you havr to rebase again |
* We removed the activity finishing triggered by pressing the back button in the reader fragment. We initially added this when we were converting CustomReader to a fragment, as we encountered crashing issues at the time. However, it was quite some time ago, and we have since refactored our code. Now, it no longer crashes and functions smoothly without terminating the activity when the back button is pressed. We also made this change for when a user presses the back button from the download screen. This back button press is within the CustomReaderFragment, so it does not impact the functionality of the DownloadFragment.
…ment` as we were using same code twice for these methods in app and custom module.
…idden these methods and describe their functionality.
… `CustomReaderFragment`.
MohitMaliFtechiz
force-pushed
the
Issue#3507
branch
from
51ce43b to
4a96824
Compare
|
@kelson42, @gouri-panda Conflicts are resolved, @gouri-panda you can review this now. |
Fixes #3507
We removed the activity finishing triggered by pressing the back button in the reader fragment. We initially added this when we were converting CustomReader to a fragment, as we encountered crashing issues at the time. However, it was quite some time ago, and we have since refactored our code. Now, it no longer crashes and functions smoothly without terminating the activity when the back button is pressed. We also made this change for when a user presses the back button from the download screen. This back button press is within the CustomReaderFragment, so it does not impact the functionality of the DownloadFragment.
backpressedissuefixed.mp4
openSearchItem, andfindInPagefunctionality to ourCoreReaderFragmentas we used the same code twice for these methods in the app and custom module.