Skip to content
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.

Sign up for GitHub

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

Jump to bottom

JetStream and Catalog updates #165

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+162 −150
Open

JetStream and Catalog updates #165

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cc17bb0
added adaptive banner
Apr 26, 2024
d3ca67c
row should span width
Feb 23, 2024
d82d404
using an intermediate variable causes an unnecessary recomposition; t…
Feb 23, 2024
d87a583
add a bit of window inset to prevent the focus from jumping to the to…
Feb 23, 2024
078ab77
use a single component in the main grid and group WIP screens together
Feb 24, 2024
9fca22a
remember focus state and restore focus with LaunchedEffect
Apr 24, 2024
70bfda9
reordered parameters
Apr 24, 2024
1f3ef36
fixed missing imports
Apr 29, 2024
3650f67
replaced windowInsetsPadding with regular padding
Apr 29, 2024
34dd960
migrated to tv-material beta01
May 8, 2024
dd1eb7c
adjusted padding and keep AppBar a fixed height
May 10, 2024
0859aeb
require onClick and return with the caller's type
May 10, 2024
e9a2983
latest AGP
May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions JetStreamCompose/jetstream/src/main/res/drawable/app_banner_foreground.xml
View file
Open in desktop
@@ -0,0 +1,34 @@
<!--
Copyright 2023 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="640dp"
android:height="360dp"
android:viewportWidth="160"
android:viewportHeight="90">
<group android:scaleX="0.2371875"
android:scaleY="0.2371875"
android:translateX="42.05"
android:translateY="23.653126">
<path
android:pathData="M135.83,60.5C135.83,47.16 146.66,36.33 160,36.33C173.34,36.33 184.17,47.16 184.17,60.5C184.17,73.84 173.34,84.67 160,84.67C146.66,84.67 135.83,73.84 135.83,60.5ZM169.67,60.5L155.17,49.62V71.37L169.67,60.5Z"
android:fillColor="#E3E2E6"
android:fillType="evenOdd"/>
<path
android:pathData="M51.58,140.82C50.58,140.82 49.66,140.68 48.83,140.4C48.02,140.13 47.29,139.76 46.65,139.32C46.01,138.87 45.47,138.39 45.02,137.88C44.59,137.34 44.25,136.84 44,136.37L46.43,133.84C46.79,134.4 47.15,134.89 47.52,135.32C47.9,135.72 48.28,136.07 48.67,136.37C49.07,136.65 49.5,136.86 49.95,137.01C50.42,137.14 50.92,137.2 51.45,137.2C52.37,137.2 53.16,137 53.82,136.6C54.5,136.17 55.03,135.6 55.39,134.9C55.75,134.2 55.93,133.41 55.93,132.53V121.72H51.26V118.1H63.55V121.72H59.77V132.92C59.77,134 59.58,135.03 59.2,135.99C58.83,136.93 58.29,137.77 57.56,138.52C56.86,139.24 56,139.81 54.97,140.21C53.97,140.62 52.84,140.82 51.58,140.82ZM69.44,140.5V118.1H85.67V121.72H73.28V136.88H85.73V140.5H69.44ZM71.33,130.8V127.25H83.97V130.8H71.33ZM98.28,140.5V121.75H91.01V118.1H109.57V121.75H102.15V140.5H98.28ZM135.6,140.82C134.15,140.82 132.83,140.66 131.63,140.34C130.44,140.02 129.35,139.54 128.37,138.9C127.41,138.24 126.54,137.44 125.78,136.5L128.21,133.46C129.42,134.91 130.63,135.93 131.82,136.53C133.02,137.11 134.35,137.4 135.82,137.4C136.68,137.4 137.49,137.29 138.26,137.08C139.02,136.84 139.63,136.5 140.08,136.05C140.55,135.58 140.78,135.03 140.78,134.39C140.78,133.96 140.68,133.59 140.46,133.27C140.25,132.95 139.95,132.67 139.57,132.44C139.18,132.2 138.74,132 138.22,131.83C137.71,131.64 137.18,131.48 136.62,131.35C136.07,131.22 135.48,131.11 134.86,131.03C133.6,130.82 132.47,130.53 131.47,130.16C130.47,129.8 129.63,129.34 128.94,128.79C128.28,128.21 127.77,127.56 127.41,126.84C127.07,126.09 126.89,125.24 126.89,124.28C126.89,123.29 127.12,122.41 127.57,121.62C128.01,120.81 128.63,120.12 129.42,119.54C130.23,118.96 131.17,118.53 132.24,118.23C133.31,117.91 134.46,117.75 135.7,117.75C137.04,117.75 138.26,117.9 139.34,118.2C140.43,118.5 141.39,118.93 142.22,119.51C143.05,120.08 143.75,120.79 144.3,121.62L141.81,124.28C141.32,123.59 140.74,123.02 140.08,122.55C139.44,122.08 138.75,121.74 138,121.52C137.25,121.29 136.46,121.17 135.63,121.17C134.71,121.17 133.9,121.29 133.2,121.52C132.49,121.76 131.94,122.09 131.54,122.52C131.13,122.92 130.93,123.43 130.93,124.05C130.93,124.52 131.04,124.94 131.28,125.3C131.51,125.64 131.85,125.95 132.3,126.23C132.77,126.48 133.34,126.71 134,126.9C134.68,127.09 135.44,127.25 136.27,127.38C137.49,127.59 138.62,127.87 139.66,128.21C140.71,128.53 141.62,128.94 142.38,129.43C143.15,129.9 143.74,130.48 144.14,131.19C144.57,131.89 144.78,132.72 144.78,133.68C144.78,135.13 144.4,136.4 143.63,137.49C142.88,138.56 141.82,139.38 140.43,139.96C139.07,140.53 137.46,140.82 135.6,140.82ZM156.56,140.5V121.75H149.29V118.1H167.85V121.75H160.43V140.5H156.56ZM173.97,140.5V118.1H184.98C186.35,118.1 187.58,118.41 188.69,119.03C189.8,119.65 190.68,120.49 191.32,121.56C191.96,122.62 192.28,123.83 192.28,125.17C192.28,126.45 191.93,127.64 191.25,128.72C190.59,129.79 189.68,130.65 188.53,131.32C187.4,131.96 186.14,132.28 184.76,132.28H177.78V140.5H173.97ZM188.69,140.5L182.23,130.39L186.26,129.59L193.37,140.53L188.69,140.5ZM177.78,128.76H184.69C185.4,128.76 186.01,128.61 186.55,128.31C187.08,127.99 187.51,127.56 187.83,127.03C188.15,126.47 188.31,125.86 188.31,125.2C188.31,124.52 188.13,123.91 187.76,123.38C187.42,122.85 186.94,122.43 186.32,122.13C185.71,121.83 185,121.68 184.21,121.68H177.78V128.76ZM200.29,140.5V118.1H216.51V121.72H204.13V136.88H216.57V140.5H200.29ZM202.17,130.8V127.25H214.81V130.8H202.17ZM221.44,140.5L231.04,118.1H235.01L244.55,140.5H240.42L234.53,126.68C234.4,126.38 234.24,125.98 234.05,125.49C233.88,124.98 233.69,124.45 233.47,123.89C233.28,123.34 233.09,122.8 232.9,122.29C232.73,121.78 232.59,121.36 232.48,121.04L233.38,121.01C233.23,121.46 233.07,121.94 232.9,122.45C232.73,122.94 232.54,123.46 232.32,123.99C232.13,124.5 231.94,124.99 231.75,125.46C231.55,125.93 231.38,126.37 231.23,126.77L225.38,140.5H221.44ZM225.7,135.32L227.14,131.83H238.56L239.71,135.32H225.7ZM250.22,140.5V118.1H253.9L262.86,131.12L261.01,131.09L270.06,118.1H273.65V140.5H269.81V131.64C269.81,129.78 269.85,128.08 269.93,126.55C270.04,124.99 270.21,123.43 270.45,121.88L270.89,123.25L262.93,134.26H260.82L253.01,123.28L253.42,121.88C253.66,123.39 253.82,124.92 253.9,126.45C254.01,127.97 254.06,129.7 254.06,131.64V140.5H250.22Z"
android:fillColor="#E3E2E6"/>
</group>
</vector>
3 changes: 0 additions & 3 deletions JetStreamCompose/jetstream/src/main/res/drawable/ic_launcher_foreground.xml
View file
Open in desktop
Expand Up @@ -23,9 +23,6 @@
android:scaleY="0.2371875"
android:translateX="16.05"
android:translateY="32.653126">
<path
android:pathData="M0,0h320v180h-320z"
android:fillColor="#1A1C1E"/>
<path
android:pathData="M135.83,60.5C135.83,47.16 146.66,36.33 160,36.33C173.34,36.33 184.17,47.16 184.17,60.5C184.17,73.84 173.34,84.67 160,84.67C146.66,84.67 135.83,73.84 135.83,60.5ZM169.67,60.5L155.17,49.62V71.37L169.67,60.5Z"
android:fillColor="#E3E2E6"
Expand Down
21 changes: 21 additions & 0 deletions JetStreamCompose/jetstream/src/main/res/mipmap-anydpi-v26/app_banner.xml
View file
Open in desktop
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright 2023 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<adaptive-icon xmlns:android="http://schemas.android.com/apk/res/android">
<background android:drawable="@color/ic_launcher_background"/>
<foreground android:drawable="@drawable/app_banner_foreground"/>
</adaptive-icon>
2 changes: 0 additions & 2 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/App.kt
View file
Open in desktop
Expand Up @@ -12,14 +12,12 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.RectangleShape
import androidx.compose.ui.graphics.toArgb
import androidx.compose.ui.unit.LayoutDirection
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.MaterialTheme
import androidx.tv.material3.Surface
import com.google.tv.material.catalog.colorutils.Scheme.Companion.dark
import com.google.tv.material.catalog.colorutils.Scheme.Companion.light
import com.google.tv.material.catalog.colorutils.toColorScheme

@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
fun App() {
var themeMode by remember { mutableStateOf(Mode.Dark) }
Expand Down
8 changes: 4 additions & 4 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/AppBar.kt
View file
Open in desktop
Expand Up @@ -22,7 +22,6 @@ import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp
import androidx.navigation.compose.currentBackStackEntryAsState
import androidx.tv.material3.Button
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.Icon
import androidx.tv.material3.MaterialTheme
import androidx.tv.material3.Text
Expand Down Expand Up @@ -64,7 +63,6 @@ fun AppBar(
}
}

@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
private fun HeadlineContent(
title: String,
Expand Down Expand Up @@ -110,7 +108,6 @@ private fun HeadlineContent(
}
}

@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
private fun Actions(
onThemeColorModeClick: () -> Unit,
Expand All @@ -129,7 +126,10 @@ private fun Actions(
)
)

Row(horizontalArrangement = Arrangement.spacedBy(16.dp)) {
Row(
horizontalArrangement = Arrangement.spacedBy(16.dp),
modifier = Modifier.padding(0.dp, 0.dp, 8.dp, 0.dp)
pflammertsma marked this conversation as resolved.
Show resolved Hide resolved
) {
actions.forEach {
Button(onClick = it.onClick) {
Icon(
Expand Down
24 changes: 12 additions & 12 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/Component.kt
View file
Open in desktop
@@ -1,34 +1,34 @@
package com.google.tv.material.catalog

val foundations = listOf(
Component.Color,
Component.Typography,
Component.Motion,
Component.Interaction,
)

val components = listOf(
Component.Button,
Component.Card,
Component.Chip,
Component.ListItem,
Component.ImmersiveList,
Component.FeaturedCarousel,
Component.NavigationDrawer,
Component.TabRow,
)

val componentsPlanned = listOf(
Component.NavigationDrawer,
pflammertsma marked this conversation as resolved.
Show resolved Hide resolved
Component.ModalDialog,
Component.TextField,
Component.MediaPlayerOverlay,
)

val foundations = listOf(
Foundation.Color,
Foundation.Typography,
Foundation.Motion,
Foundation.Interaction,
)

enum class Foundation(val title: String, val imageArg: String, val routeValue: String) {
enum class Component(val title: String, val imageArg: String, val routeValue: String) {
Color(title = "Color", imageArg = "colors", routeValue = NavGraph.Color.routeName),
Typography(title = "Typography", imageArg = "typography", routeValue = NavGraph.Typography.routeName),
Motion(title = "Motion", imageArg = "motion", routeValue = NavGraph.Motion.routeName),
Interaction(title = "Interaction", imageArg = "interaction", routeValue = NavGraph.Interaction.routeName),
}

enum class Component(val title: String, val imageArg: String, val routeValue: String) {
Button(title = "Buttons", imageArg = "buttons", routeValue = NavGraph.Buttons.routeName),
Card(title = "Cards", imageArg = "cards", routeValue = NavGraph.Cards.routeName),
Chip(title = "Chips", imageArg = "chips", routeValue = NavGraph.Chips.routeName),
Expand Down
12 changes: 6 additions & 6 deletions ...og/app/src/main/java/com/google/tv/material/catalog/ExamplesScreenWithDottedBackground.kt
View file
Open in desktop
Expand Up @@ -30,15 +30,14 @@ import androidx.compose.ui.focus.onFocusChanged
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.PathEffect
import androidx.compose.ui.unit.dp
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.ListItem
import androidx.tv.material3.MaterialTheme
import androidx.tv.material3.Text
import androidx.tv.material3.surfaceColorAtElevation
import kotlinx.coroutines.delay
import kotlin.time.Duration.Companion.milliseconds

@OptIn(ExperimentalTvMaterial3Api::class, ExperimentalComposeUiApi::class)
@OptIn(ExperimentalComposeUiApi::class)
@Composable
fun ExamplesScreenWithDottedBackground(
actions: List<ExampleAction>
Expand Down Expand Up @@ -76,10 +75,11 @@ fun ExamplesScreenWithDottedBackground(
if (it.isFocused) {
activeAction = action
}
}
) {
Text(text = action.title)
}
},
headlineContent = {
Text(text = action.title)
}
)
}
}
}
Expand Down
21 changes: 10 additions & 11 deletions ...g/app/src/main/java/com/google/tv/material/catalog/FontScaleAndLayoutDirectionSelector.kt
View file
Open in desktop
Expand Up @@ -28,13 +28,12 @@ import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.tv.foundation.PivotOffsets
import androidx.tv.foundation.lazy.list.TvLazyColumn
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.Icon
import androidx.tv.material3.ListItem
import androidx.tv.material3.MaterialTheme
import androidx.tv.material3.NonInteractiveSurfaceDefaults
import androidx.tv.material3.RadioButton
import androidx.tv.material3.Surface
import androidx.tv.material3.SurfaceDefaults
import androidx.tv.material3.Text

@OptIn(ExperimentalTvMaterial3Api::class, ExperimentalComposeUiApi::class)
Expand Down Expand Up @@ -69,7 +68,7 @@ fun FontScaleAndLayoutDirectionSelector(
.width(300.dp)
.padding(12.dp)
.align(Alignment.End),
colors = NonInteractiveSurfaceDefaults
colors = SurfaceDefaults
.colors(containerColor = MaterialTheme.colorScheme.surface)
) {
TvLazyColumn(
Expand Down Expand Up @@ -122,10 +121,10 @@ fun FontScaleAndLayoutDirectionSelector(
},
trailingContent = {
RadioButton(selected = isSelected, onClick = { })
}
) {
Text(text = it.title)
}
},
headlineContent = {
Text(text = it.title)
})
}
}
}
Expand Down Expand Up @@ -163,10 +162,10 @@ fun FontScaleAndLayoutDirectionSelector(
},
trailingContent = {
RadioButton(selected = isSelected, onClick = { })
}
) {
Text(text = it.title)
}
},
headlineContent = {
Text(text = it.title)
})
}
}
}
Expand Down
32 changes: 27 additions & 5 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/HomeGrid.kt
View file
Open in desktop
Expand Up @@ -6,19 +6,30 @@ import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.focusRequester
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: redundant import

import androidx.compose.ui.focus.focusRestorer
import androidx.compose.ui.unit.dp
import androidx.tv.foundation.lazy.grid.TvGridCells
import androidx.tv.foundation.lazy.grid.TvGridItemSpan
import androidx.tv.foundation.lazy.grid.TvLazyVerticalGrid
import androidx.tv.foundation.lazy.grid.itemsIndexed
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.Text

@OptIn(ExperimentalTvMaterial3Api::class, ExperimentalComposeUiApi::class)
@OptIn(ExperimentalComposeUiApi::class)
@Composable
fun HomeGrid() {
val focusRequester = remember { FocusRequester() }
val itemClick = {
focusRequester.saveFocusedChild()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this. If the item gains focus, it automatically gets saved if the parent grid is wrapped with "focusRestorer"

}
LaunchedEffect(Unit) {
focusRequester.requestFocus()
}
Comment on lines +30 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could lead to crashes because of race condition. It is possible that the LaunchedEffect is executed before the items are placed and it would cause a crash. We can make use of onPlaced or onGloballyPositioned modifiers to see if the item is available to get focus. Refer b/276738340#comment6

Column(Modifier.fillMaxSize()) {
Column(
Modifier
Expand All @@ -27,7 +38,10 @@ fun HomeGrid() {
) {
TvLazyVerticalGrid(
columns = TvGridCells.Fixed(4),
modifier = Modifier.padding(top = 12.dp),
modifier = Modifier
.padding(top = 12.dp)
.focusRequester(focusRequester)
.focusRestorer(),
Comment on lines +43 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to make use of focusRequester modifier to save and restore focus when using focusRestorer modifier.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The focusRequester modifier is necessary as the request focus method is called over the associated FocusRequester object. Without the modifier, calling requestFocus method crashes the app.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, focusRestorer doesn't need any focusRequester modifier, to work. If the app is crashing, it could be a bug, most likely it would be because of the 3rd point: "attempting to request focus during composition".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that there is a Launched Effect which is requesting focus on the focus requester. If the focusRequester is not initialized, it will crash the app. I think the goal here is to request focus on the first element. In that case, the focusRequester should be assigned to the first item's modifier and not the parent grid.

contentPadding = PaddingValues(vertical = 12.dp),
horizontalArrangement = Arrangement.spacedBy(20.dp),
verticalArrangement = Arrangement.spacedBy(20.dp)
Expand All @@ -37,15 +51,23 @@ fun HomeGrid() {
}

itemsIndexed(foundations) { index, item ->
FoundationsGridCard(foundation = item)
ComponentsGridCard(component = item, onClick = itemClick)
}

item(span = { TvGridItemSpan(4) }) {
Text(text = "Components")
}

itemsIndexed(components) { index, item ->
ComponentsGridCard(component = item)
ComponentsGridCard(component = item, onClick = itemClick)
}

item(span = { TvGridItemSpan(4) }) {
Text(text = "Components (planned)")
}

itemsIndexed(componentsPlanned) { index, item ->
ComponentsGridCard(component = item, onClick = itemClick)
}
}
}
Expand Down
47 changes: 10 additions & 37 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/HomeGridCard.kt
View file
Open in desktop
Expand Up @@ -7,26 +7,28 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.unit.dp
import androidx.tv.material3.Card
import androidx.tv.material3.CardDefaults
import androidx.tv.material3.CardLayoutDefaults
import androidx.tv.material3.ExperimentalTvMaterial3Api
import androidx.tv.material3.StandardCardLayout
import androidx.tv.material3.StandardCardContainer
import androidx.tv.material3.Text

@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
fun ComponentsGridCard(
component: Component,
modifier: Modifier = Modifier
modifier: Modifier = Modifier,
onClick: (() -> Any)? = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = {}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adjusted this slightly to allow the lambda to return Any and make sure the invoker returns it as expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callback should not have a return value, as it is to deliver click events to the outer component.

If we need to handle the result of function calls that potentially fails, we should introduce another component to manage state as recommended in this doc.

) {
val image = getHomeGridCardImage(imageArg = component.imageArg)
val navHostController = LocalNavController.current

StandardCardLayout(
StandardCardContainer(
modifier = modifier,
imageCard = {
CardLayoutDefaults.ImageCard(
onClick = { navHostController.navigate(component.routeValue) },
Card(
onClick = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code to handle click events should be updated as follows:

Card(
   onClick = {
     onClick()
     navHostController.navigate(component.routeValue)
  }

onClick?.invoke()
pflammertsma marked this conversation as resolved.
Show resolved Hide resolved
navHostController.navigate(component.routeValue)
},
interactionSource = it,
colors = CardDefaults.colors(containerColor = Color.Transparent)
) {
Expand All @@ -41,32 +43,3 @@ fun ComponentsGridCard(
}
)
}

@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
fun FoundationsGridCard(
foundation: Foundation,
modifier: Modifier = Modifier
) {
val image = getHomeGridCardImage(imageArg = foundation.imageArg)
val navHostController = LocalNavController.current

StandardCardLayout(
modifier = modifier,
imageCard = {
CardLayoutDefaults.ImageCard(
onClick = { navHostController.navigate(foundation.routeValue) },
interactionSource = it,
colors = CardDefaults.colors(containerColor = Color.Transparent)
) {
Image(painter = painterResource(id = image), contentDescription = null)
}
},
title = {
Text(
text = foundation.title,
modifier = Modifier.padding(top = 8.dp)
)
}
)
}