JetStream and Catalog updates #165
Conversation
(cherry picked from commit 4a39926)
…he lambda provides the selected tab index
…p of the screen while navigating to the right
pflammertsma
assigned pflammertsma and chikoski and unassigned chikoski and pflammertsma
TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/AppBar.kt
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
| val componentsPlanned = listOf( | ||
| Component.NavigationDrawer, |
There was a problem hiding this comment.
NavigationDrawer is part of Beta right?
There was a problem hiding this comment.
Yes, but this is just a placeholder and the implementation is not there yet.
| Row(horizontalArrangement = Arrangement.spacedBy(16.dp)) { | ||
| Row( | ||
| horizontalArrangement = Arrangement.spacedBy(16.dp), | ||
| modifier = Modifier.padding(0.dp, 0.dp, 8.dp, 0.dp) |
There was a problem hiding this comment.
nit: While reading the code, It is hard to guess what side this padding is applied to. We could make use of named arguments like: Modifier.padding(bottom = 8.dp) or Modifier.padding(right = 8.dp) and this way, we won't have to set the other values to 0, explicitly.
| ) | ||
|
|
||
| val componentsPlanned = listOf( | ||
| Component.NavigationDrawer, |
| .focusRequester(focusRequester) | ||
| .focusRestorer(), |
There was a problem hiding this comment.
We don't need to make use of focusRequester modifier to save and restore focus when using focusRestorer modifier.
There was a problem hiding this comment.
Removing this gives me an exception:
java.lang.IllegalStateException:
FocusRequester is not initialized. Here are some possible fixes:
1. Remember the FocusRequester: val focusRequester = remember { FocusRequester() }
2. Did you forget to add a Modifier.focusRequester() ?
3. Are you attempting to request focus during composition? Focus requests should be made in
response to some event. Eg Modifier.clickable { focusRequester.requestFocus() }
| onClick = { navHostController.navigate(component.routeValue) }, | ||
| Card( | ||
| onClick = { | ||
| onClick?.invoke() |
There was a problem hiding this comment.
nit: The following code can be simpler and easier to understand:
if(onClick != null) {
onClick()
}| @Composable | ||
| fun ComponentsGridCard( | ||
| component: Component, | ||
| modifier: Modifier = Modifier | ||
| modifier: Modifier = Modifier, | ||
| onClick: (() -> Any)? = null |
There was a problem hiding this comment.
Would you elaborate on this? I believe click event handler does not need to return value, so the type of onClick parameter should be as follows. The suggested code would simplify the code to call the callback:
onClick: () -> Unit = {}| @@ -1,13 +1,13 @@ | |||
| [versions] | |||
| agp = "8.3.1" | |||
| agp = "8.3.2" | |||
There was a problem hiding this comment.
We should update AGP version to "8.4.0" as Android Studio Jellyfish became stable.
JetStream:
TV Material Catalog: