Improve ShowkaseBrowser codebase and update navigation stack #390
Conversation
| shape = MaterialTheme.shapes.large | ||
| // This is added to make sure that the navigation of the ShowkaseBrowser does not break | ||
| // when one of the previews has a back press handler in the implementation of the component. | ||
| val backPressedDispatcherOwner = rememberOnBackPressedDispatcherOwner() |
There was a problem hiding this comment.
Doing this at the top level (which was previously the case) was causing the navigation to not work at all. Instead, I moved this down to specifically happen for the Composables that are responsible for rendering the component previews as that's where we want to override/intercept any back handling that the previews might be doing themselves. Doing so fixed the issue but I'm not 100% on why doing this at the top level didn't work okay.
| ShowkaseCategory.TYPOGRAPHY -> navController.navigate( | ||
| ShowkaseCurrentScreen.TYPOGRAPHY_GROUPS | ||
| ) | ||
| ShowkaseCategory.COMPONENTS -> onNavigateToComponentGroups() |
There was a problem hiding this comment.
It would be nice if we could check here whether the component group only has one component and directly navigate into it.
| groupedComponentMap = groupedComponentMap, | ||
| showkaseBrowserScreenMetadata = showkaseBrowserScreenMetadata, | ||
| onUpdateShowkaseBrowserScreenMetadata = onUpdateShowkaseBrowserScreenMetadata, | ||
| navigateTo = navigateTo, | ||
| ) | ||
| } | ||
| composable(ShowkaseCurrentScreen.COMPONENT_STYLES.name) { | ||
| ShowkaseComponentStylesScreen( |
There was a problem hiding this comment.
Similarly, skipping the styles screen when there's only one style would be super nice. Directly to the detail screen.
| @@ -61,7 +61,8 @@ dependencies { | |||
| // Support Libraries | |||
| implementation deps.support.appCompat | |||
| implementation deps.support.ktx | |||
| implementation deps.support.lifecycleExtensions | |||
| implementation deps.support.lifecycleComposeViewModel | |||
| implementation deps.support.lifecycleComposeRuntime | |||
| @@ -62,7 +62,8 @@ dependencies { | |||
| // Support Libraries | |||
| implementation deps.support.appCompat | |||
| implementation deps.support.ktx | |||
| implementation deps.support.lifecycleExtensions | |||
| implementation deps.support.lifecycleComposeViewModel | |||
| implementation deps.support.lifecycleComposeRuntime | |||
| @@ -94,7 +94,8 @@ dependencies { | |||
| // Support Libraries | |||
| implementation deps.support.appCompat | |||
| implementation deps.support.ktx | |||
| implementation deps.support.lifecycleExtensions | |||
| implementation deps.support.lifecycleComposeViewModel | |||
| implementation deps.support.lifecycleComposeRuntime | |||
The browser side of Showkase was written in ~March 2020 when documentation for Compose was non-existant and best practices weren't documented. A lot of the code did not meet the current standards and was causing breaking behavior when trying to update some libraries. I ended up going down a rabbit hole and decided to fix some of the obvious issues, although I can see some more obvious improvements that I'll do in the future (or would love someone to help and contribute if you are interested).
@airbnb/showkase-maintainers