-
-
Notifications
You must be signed in to change notification settings - Fork 402
Migrate Count Inputs to Jetpack Compose #6282
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
Migrate Count Inputs to Jetpack Compose #6282
Conversation
westnordost
left a comment
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.
Very nice!
There's furthermore with the identical UI (with each different icon):
- Charging station capacity
- Motorcycle parking capacity
app/src/main/java/de/westnordost/streetcomplete/quests/step_count/StepCountForm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/CountForm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/CountForm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/CountForm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/CountForm.kt
Outdated
Show resolved
Hide resolved
# Conflicts: # app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/bike_parking_capacity/BikeParkingCapacityForm.kt # app/src/androidMain/kotlin/de/westnordost/streetcomplete/ui/common/CountInput.kt # app/src/androidMain/res/layout/compose_view.xml # app/src/androidMain/res/layout/quest_step_count.xml
|
So, seems almost done, only are missing. |
…t-form-composable # Conflicts: # app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AddCountInput.kt
They are done now |
app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/step_count/AddStepCountForm.kt
Show resolved
Hide resolved
westnordost
left a comment
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 like how it is now less lines of code than before. :-)
I fixed some things:
- Dark mode was wrong (missing surface, missing application of app theme)
- also, it just crashed when trying to open the form. It would be good if you tested your work before requesting a review.
- public
@Composables should always have aModifieras parameter - Naming convention for Abstract quest forms is to have that start with (another)
A - parametrized the text style
- after all, I defined a new text style to better describe what it should be used for (-> input fields that are alone in the quest forms, i.e. the only input field. So, they can be very very large)
- finally, what really bugged me during final test, was, that the input fields all started out with a pre-filled "0". So, to input a number, one would think one needs to first delete the 0, then add the number. But this doesn't even work, as the input only accepted ints, not even an empty string. So I changed that. Empty string is fine, too (but doesn't count as "form is filled in")
|
From my side, this could be merged. Would you like to have another look at the changes I made before I merge it? |
|
Yes, I've looked it up and it seems fairly complete now! For testing, the emulator was extremely laggy and I couldn't manage to open a quest. Do you use any settings to improve emulation performance? |
|
Yeah, that's a MapLibre problem. The map is super laggy, but you should be able to open the menu -> settings -> show quest forms. No need to deal with the map. |
|
Also, what might help is to set to Graphics Acceleration -> Hardware in the Additional Settings tag when you edit the emulated device. |
I've migrated the following widgets to Jetpack Compose:
I've added Preview for these two Forms.
Let me know if there is any other count input I forgot to migrate to Compose.