-
Notifications
You must be signed in to change notification settings - Fork 1
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
Improve score widget #106
base: main
Are you sure you want to change the base?
Improve score widget #106
Conversation
@mdrlzy Could you brief which improvements were made in this PR? |
scorewidget/build.gradle.kts
Outdated
implementation(libraries.arklib) | ||
implementation(libraries.orbit.mvi.viewmodel) | ||
implementation(libs.orbit.mvi.compose) | ||
implementation("com.github.mdrlzy:ComposeCounterSlider:0.1.4") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this dependency be added in predefined version management scheme that we already have?
Looks awesome! One bug, not critical since it's just a sample:
|
) = lifecycleScope.launch { | ||
val (index, scoreStorage) = setupIndexAndScoreStorage(root) | ||
val id = index.allPaths().toList() | ||
.find { it.second == resourcePath }!!.first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result of find
could be null
, and app will crash in that case because of !!
assertion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. This can happen if file is selected from outside root
root: Path, | ||
resourcePath: Path | ||
) = lifecycleScope.launch { | ||
val (index, scoreStorage) = setupIndexAndScoreStorage(root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setupIndexAndScoreStorage
is a suspend
function so it should be scheduled in background thread. It's now running in main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was sure that initialization of Index
and ScoreStorage
occurs on Dispatcher.IO
, but ScoreStorage
does not change dispatcher :/
Replace old score view with plain arrows with new one with animations
Before
After
scorewidgetnew.mp4