Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix media span and task list memory leaks #1088

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

khaykov
Copy link
Member

@khaykov khaykov commented Jul 25, 2024

Fix

This PR fixes memory leaks in media and task list spans.

Test

  1. Toggle items in task list, make sure they work as expected.
  2. Delete images, confirm that you see a "media deleted" snackbar.
  3. Perform same actions after changing device orientation or bringing up to foreground from background.

@khaykov khaykov requested review from danilo04, zwarm and AmandaRiu July 25, 2024 15:54
@zwarm zwarm self-assigned this Jul 25, 2024
Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

@khaykov - Tested and all works as expected. 👍

I have one question: Might it be safer to use
val drawable = contextRef.get()?.resources?.getDrawable(R.drawable.ic_checkbox, null)

instead of

val d: Drawable = contextRef.get()!!.resources.getDrawable(R.drawable.ic_checkbox, null)

I am approving the PR, but will leave the merge for you so you'll have a chance to review my question.

@khaykov
Copy link
Member Author

khaykov commented Jul 25, 2024

@zwarm Good point! Switched to elvis operator 👍

@khaykov khaykov merged commit 2d66175 into trunk Jul 25, 2024
14 checks passed
@khaykov khaykov deleted the issue/fix-media-span-and-task-list-memory-leaks branch July 25, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants