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
Open

JetStream and Catalog updates #165

Show file tree
Hide file tree
Changes from 7 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>
7 changes: 6 additions & 1 deletion TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/AppBar.kt
View file
Open in desktop
Expand Up @@ -6,9 +6,11 @@ import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.windowInsetsPadding
import androidx.compose.foundation.shape.CircleShape
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
Expand Down Expand Up @@ -129,7 +131,10 @@ private fun Actions(
)
)

Row(horizontalArrangement = Arrangement.spacedBy(16.dp)) {
Row(
horizontalArrangement = Arrangement.spacedBy(16.dp),
modifier = Modifier.windowInsetsPadding(WindowInsets(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
26 changes: 23 additions & 3 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/HomeGrid.kt
View file
Open in desktop
Expand Up @@ -6,6 +6,8 @@ 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.unit.dp
Expand All @@ -19,6 +21,13 @@ import androidx.tv.material3.Text
@OptIn(ExperimentalTvMaterial3Api::class, 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 +36,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 +49,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
37 changes: 6 additions & 31 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/HomeGridCard.kt
View file
Open in desktop
Expand Up @@ -17,7 +17,8 @@ import androidx.tv.material3.Text
@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
Expand All @@ -26,7 +27,10 @@ fun ComponentsGridCard(
modifier = modifier,
imageCard = {
CardLayoutDefaults.ImageCard(
onClick = { navHostController.navigate(component.routeValue) },
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 +45,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)
)
}
)
}
12 changes: 6 additions & 6 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/NavGraph.kt
View file
Open in desktop
Expand Up @@ -147,12 +147,6 @@ enum class NavGraph(
}
}
),
NavigationDrawer(
routeName = "nav-drawer",
composable = {
WorkInProgressScreen()
}
),
TabRow(
routeName = "tab-row",
composable = { appBar ->
Expand All @@ -162,6 +156,12 @@ enum class NavGraph(
}
}
),
NavigationDrawer(
routeName = "nav-drawer",
composable = {
WorkInProgressScreen()
}
),
ModalDialog(
routeName = "modal-dialog",
composable = {
Expand Down
2 changes: 2 additions & 0 deletions ...alCatalog/app/src/main/java/com/google/tv/material/catalog/screens/ImmersiveListScreen.kt
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@ import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.width
Expand Down Expand Up @@ -99,6 +100,7 @@ fun ImmersiveListScreen() {
TvLazyRow(
modifier = Modifier
.align(Alignment.BottomStart)
.fillMaxWidth()
.padding(bottom = 20.dp)
.focusRestorer { firstChildFr },
contentPadding = PaddingValues(start = 58.dp),
Expand Down
6 changes: 1 addition & 5 deletions TvMaterialCatalog/app/src/main/java/com/google/tv/material/catalog/screens/TabRowScreen.kt
View file
Open in desktop
Expand Up @@ -130,12 +130,8 @@ private fun UnderlinedIndicatorTabRow() {

@Composable
private fun TabPanels(selectedTabIndex: Int) {
var value = 0

AnimatedContent(targetState = selectedTabIndex, label = "") {
value = it

when (selectedTabIndex) {
when (it) {
getTabIndex("Search") -> {
Column(
modifier = Modifier
Expand Down