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

Don't block UI thread when opening tile + shortcuts settings #2450

Merged

Conversation

jpelgrom
Copy link
Member

@jpelgrom jpelgrom commented Apr 12, 2022

Summary

Did you ever notice that opening the tile and shortcuts settings takes a little longer than other settings fragments? Turns out it is because the app was loading all the icons for the icon picker on the main thread, every time the fragment resumed. This PR fixes that by moving it to a background thread and only doing it once.

(also cleaning up some annotations for the shortcuts fragment)

Screenshots

n/a

Link to pull request in Documentation repository

n/a

Any other notes

 - Don't load the icon dialog's icon pack every time the fragment is resumed, it only needs to be loaded once
 - Don't load the icon dialog's icon pack on the main thread, instead load it on the IO thread to prevent blocking the fragment UI thread (as suggested in the loadDrawables function)
 - Clean up @RequiresApi(Build.VERSION_CODES.N_MR1) annotations
Copy link
Collaborator

@JBassett JBassett left a comment

Choose a reason for hiding this comment

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

This brings up the question, are we doing this on widgets and notifications?

@JBassett JBassett merged commit 4a5d580 into home-assistant:master Apr 12, 2022
@jpelgrom jpelgrom deleted the dont-block-ui-tiles-shortcuts branch April 13, 2022 05:58
@jpelgrom
Copy link
Member Author

This brings up the question, are we doing this on widgets and notifications?

Notifications icons with mdi: don't use the icon picker resources because no picker is needed, instead it uses the iconics library which seems to perform much better (probably because it doesn't need to load everything, only look up by ID).

The button widget does use the icon picker and loads icons on the main thread (at least it is only loaded once). It should probably be improved but I didn't think about the actual widget itself, and the configuration activity includes more blocking calls in onCreate so I think it is better to make the icon dialog in the activity part of a larger refactoring PR and/or conversion to Compose.

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.

4 participants