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

[SES-2512] Rewrite ProfilePictureView #1622

Merged
merged 7 commits into from
Aug 19, 2024
Merged

[SES-2512] Rewrite ProfilePictureView #1622

merged 7 commits into from
Aug 19, 2024

Conversation

simophin
Copy link

@simophin simophin commented Aug 15, 2024

This PR rewrites the ProfilePictureView so that it defers the db access to non-UI threads.

// to launch a coroutine from a view. The potential memory leak is not a concern here, as
// the coroutine is very short-lived. If you change the code here to be long live then you'll
// need to find a better scope to launch the coroutine from.
lastLoadJob = GlobalScope.launch(Dispatchers.Main) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is definitely not ideal... The whole view is problematic, which is unfortunately not a new problem. We really shouldn't be having so much heavy logic in a piece of UI...
There's isn't too much else we can do with the current set up...
This will be part of the things we can fix once we move away from the xml views.

Copy link
Collaborator

@ThomasSession ThomasSession left a comment

Choose a reason for hiding this comment

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

Nice

@simophin simophin merged commit 9919f71 into dev Aug 19, 2024
2 checks passed
@simophin simophin deleted the profile-view-optimise branch August 19, 2024 07:32
simophin pushed a commit that referenced this pull request Aug 20, 2024
simophin added a commit that referenced this pull request Aug 20, 2024
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.

2 participants