refactor: mount receivers #16714
Merged
refactor: mount receivers #16714
+64
−131
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
david-allison
added
Needs Author Reply
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Arthur-Milchior
force-pushed
the
receiver
branch
from
d90bdc8
to
77f766b
Compare
mikehardy
approved these changes
Jul 10, 2024
|
@david-allison with limited time and having already reviewed it I'm not sure on the value of splitting the refactor to a commit. It appears to work in local testing, I'd merge as is but you've got a pending reject, so back to you |
mikehardy
added
Needs Second Approval
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mikehardy
added
Needs Author Reply
|
Agreed - refactor would be better as a PR |
This comment was marked as resolved.
This comment was marked as resolved.
Arthur-Milchior
force-pushed
the
receiver
branch
from
77f766b
to
f5e66cc
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
|
For posterity: d90bdc8 |
david-allison
removed
Priority-Critical
Review High Priority
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Arthur-Milchior
force-pushed
the
receiver
branch
from
cf8d770
to
34a2c9f
Compare
Arthur-Milchior
added
Needs Review
and removed
Needs Author Reply
Arthur-Milchior
force-pushed
the
receiver
branch
from
34a2c9f
to
38a39d5
Compare
lukstbit
reviewed
Sep 23, 2024
Arthur-Milchior
force-pushed
the
receiver
branch
2 times, most recently
from
8eb3512
to
b0c3b08
Compare
lukstbit
added
Needs Second Approval
david-allison
requested changes
Sep 24, 2024
david-allison
added
the
Needs Author Reply
david-allison
dismissed
mikehardy’s stale review
Reviewed changes were split out into 16717
This ensure that the same receiver appears in all activities and not just the few one where the code was essentially copy pasted. The Deck Picker also listen for mounting, so I renamed its receiver as a `mountReceiver` instead
Arthur-Milchior
force-pushed
the
receiver
branch
from
b0c3b08
to
3e86f44
Compare
david-allison
approved these changes
Sep 28, 2024
david-allison
added
Pending Merge
github-actions
bot
removed
the
Pending Merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This ensure there is no change in behaviour.
I checked whether we could try to not export the download of decks, but it does not works.
The app does not crash on api 24 on simulator.
There was a slight problem, with variables that could potentially be null. To ensure the type checker agreed with the code, without using
!!, I had to rewrite some of the logic.I also ensured that some of the code that was copy/pasted in various activity now entirely belong to AnkiActivity. As a side effect, all activities that used to listen to the "unmount" notification and finished, will now also show a toast when the card gets unmounted. Currently it was only the deck picker.