Skip to content

Comments

Fix more onBackPressed() deprecations#17579

Merged
BrayanDSO merged 5 commits intoankidroid:mainfrom
lukstbit:chore_onBackPressedDeprecation
Dec 10, 2024
Merged

Fix more onBackPressed() deprecations#17579
BrayanDSO merged 5 commits intoankidroid:mainfrom
lukstbit:chore_onBackPressedDeprecation

Conversation

@lukstbit
Copy link
Member

@lukstbit lukstbit commented Dec 9, 2024

Purpose / Description

Replace some usages of onBackPressed() with specific OnBackPressedCallbacks that are enabled/disabled ahead of time(of BACK action). More info in the commits messages. Four more usages of onBackPressed() left but those are more difficult to fix. See some videos with the new behavior(must add android:enableOnBackInvokedCallback="true" for the application tag in the manifest):

study_options.mp4
shared_decks.mp4
reviewer.mp4
card_browser.mp4
card_browser_apperance.mp4

Fixes

How Has This Been Tested?

Used the app screens, less for shared decks(because I went there too many times I'm seeing 429 responses:( ).

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

The commit messages made this trivial to review, thank you so much!

@david-allison
Copy link
Member

@david-allison david-allison added the Blocked by dependency Currently blocked by some other dependent / related change label Dec 9, 2024
@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review Blocked by dependency Currently blocked by some other dependent / related change labels Dec 10, 2024
The previous code in onBackPressed() was just setting the result code
to RESULT_OK and then finishing the activity so I set that result code
directly in onCreate(), the associated fragment can change it as it
chooses.

With this change(vs. adding a OnBackPressedCallback) we get the nice
peek animation when dragging from the left to go back(
enableOnBackInvokedCallback has to be enabled for the application in
the manifest file).
Moved the previous back handling code from the activity to the related
fragment. The predictive BACK navigation is done by changing the enabled
status of the back callback every time isDownloadInProgress is updated.

There was also a direct call to the activity onBackPressed() which was
replaced by the equivalent onBackPressedDispatcher.onBackPressed().
The previous code was showing a discard dialog if there were any
changes, meaning that the user entered something different in the
input boxed than what text was the screen initialized with.

The fix introduces two listeners for the input boxes to use them as a
dynamic listener for when there's change and implicitly could be BACK
changes as well.

The code still shows the discard changes dialog for the UP button in
the toolbar like before.
Created a back callback for closing the drawer and connected it to the
drawer's listener to enable and disable it dynamically.

The code was added to initNavigationDrawer method which all screens
that appear in the drawer(DeckPicker, CardBrowser and Reviewer) call
in onCreate().

Having both the drawer opening and the back gesture happening on
swipe from the screen's left edge isn't great.
@lukstbit lukstbit force-pushed the chore_onBackPressedDeprecation branch from 4a5543d to e398fb7 Compare December 10, 2024 06:59
Copy link
Member

@BrayanDSO BrayanDSO left a comment

Choose a reason for hiding this comment

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

Nice. Thanks!

@BrayanDSO BrayanDSO added this pull request to the merge queue Dec 10, 2024
@BrayanDSO BrayanDSO added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Dec 10, 2024
Merged via the queue into ankidroid:main with commit 66c5403 Dec 10, 2024
@github-actions github-actions bot added this to the 2.21 release milestone Dec 10, 2024
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Dec 10, 2024
@lukstbit lukstbit deleted the chore_onBackPressedDeprecation branch December 10, 2024 13:38
@mikehardy mikehardy mentioned this pull request Dec 23, 2024
5 tasks
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