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

compose previews #834

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

compose previews #834

wants to merge 2 commits into from

Conversation

vpodlesnyak
Copy link
Contributor

*/
class ApplicationHolder {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unsure whether class + companion object combination was intentional

field = application
object ApplicationHolder {

private var _applicationContext: Context? = null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

android system does not really use an Application object but relies on the Context

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it was mostly a way to use types to guarantee the context set was the actual application context and not an Activity context by accident

actual class DefaultFontLoader(private val context: Context?) : FontLoader {
actual constructor() : this(if (ApplicationHolder.isInitialized) ApplicationHolder.applicationContext else null)
actual override fun loadFont(identifier: String, defaultValue: KalugaFont?): KalugaFont? =
context?.getResource(identifier, DefType.font) { id -> ResourcesCompat.getFont(context, id) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

converted to using blocking call after all

@ci-splendo
Copy link
Contributor

Code coverage

Total Project Coverage 62.75%

defaultValue
}
}
actual constructor() : this(if (ApplicationHolder.isInitialized) ApplicationHolder.applicationContext else null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I realize this is just existing behaviour, but since you've been looking at the code here... is the idea when proceeding with null that it's for previews / unit tests or something? It's such strange behaviour.

@thoutbeckers
Copy link
Collaborator

breaks example, at least this time we know from CI :)

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