[feature/#245] Apply type safe navigation on TimetableItemDetailScreen #268
Conversation
|
Detekt check failed. Please run |
|
Detekt check failed. Please run |
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable | ||
| data class TimeTableItem( |
There was a problem hiding this comment.
Thanks. It is a very tiny point but we are using Timetable instead of TimeTable. Could you fix this point?
l2hyunwoo
force-pushed
the
feature/#245
branch
from
5bee599 to
b3c02a4
Compare
|
I add test config for type safe navigation in core/testing/src/main/java/io/github/droidkaigi/confsched/testing/rules/RobotTestRule.kt. |
l2hyunwoo
force-pushed
the
feature/#245
branch
from
d78a0ed to
07e06a1
Compare
|
Detekt check failed. Please run |
1 similar comment
|
Detekt check failed. Please run |
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable | ||
| data class TimetableItem( |
There was a problem hiding this comment.
Thank you! Now we have type-safe navigation! I think "TimetableItem" could be a little confusing when we search our code. Do you have any opinion on the naming of this? How about using "TimetableItemDetailDestination" or something similar? Is there a guide or example for best practices in naming such components?
There was a problem hiding this comment.
No, there's no proper guide of naming convention of navagtion type. Maybe, this repository will be best-practice for android developers who want to apply type-safe compose navigaiton.
I think navigation type's suffix should be "Route" or "Destination" (such as compose-destinations). So, I'll fix to "TimetableItemDestination" (Detail makes repeating words I think)
core/testing/src/main/java/io/github/droidkaigi/confsched/testing/rules/RobotTestRule.kt
Show resolved
Hide resolved
| @@ -56,7 +55,7 @@ fun NavGraphBuilder.sessionScreens( | |||
| onCalendarRegistrationClick: (TimetableItem) -> Unit, | |||
| onShareClick: (TimetableItem) -> Unit, | |||
| ) { | |||
| composable(timetableItemDetailScreenRoute) { | |||
| composable<TimetableItemDestination> { | |||
There was a problem hiding this comment.
How about using TimetableItemDetailDestination? Because we use this name to navigate the screen.
There was a problem hiding this comment.
Okay. I'll fix it.
takahirom
added
the
awesome
l2hyunwoo
force-pushed
the
feature/#245
branch
from
71427ab to
5343d81
Compare
|
Thanks for attaching awesome label. I'll do my best for preparing presentation in this conference :) |
|
Thank you for your contribution! |
Issue
Overview (Required)
Links
Movie (Optional)
Screen_recording_20240811_131911.mp4
Screen_recording_20240811_131956.mp4