From ee8b70003d69212b9dd60e075aae47e78982de5a Mon Sep 17 00:00:00 2001 From: Qamar Safadi Date: Sat, 22 Jul 2023 12:07:42 +0300 Subject: [PATCH 01/13] feat: Show loading progress for Image Component as a solve for //TODO b/228077205, --- .../component/DynamicAsyncImage.kt | 33 ++++++++++++++-- .../nowinandroid/core/ui/NewsResourceCard.kt | 38 +++++++++++++++---- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index e26a824af..abd783545 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -16,11 +16,19 @@ package com.google.samples.apps.nowinandroid.core.designsystem.component +import androidx.compose.foundation.Image +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.painter.Painter +import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** @@ -34,11 +42,30 @@ fun DynamicAsyncImage( placeholder: Painter? = null, ) { val iconTint = LocalTintTheme.current.iconTint - AsyncImage( - placeholder = placeholder, + SubcomposeAsyncImage( + error = { + if (placeholder != null) { + Image( + painter = placeholder, + contentDescription = "placeholder image", + ) + } + }, model = imageUrl, contentDescription = contentDescription, colorFilter = if (iconTint != null) ColorFilter.tint(iconTint) else null, modifier = modifier, - ) + loading = { + Box( + modifier = modifier, + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator( + Modifier.size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + }, + + ) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 009fb1249..aed1871fa 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -17,6 +17,7 @@ package com.google.samples.apps.nowinandroid.core.ui import androidx.compose.foundation.Canvas +import androidx.compose.foundation.Image import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -31,6 +32,7 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme @@ -44,6 +46,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment +import androidx.compose.ui.Alignment.Companion import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale @@ -58,6 +61,8 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.AsyncImagePainter +import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -146,20 +151,37 @@ fun NewsResourceCardExpanded( fun NewsResourceHeaderImage( headerImageUrl: String?, ) { - AsyncImage( - placeholder = if (LocalInspectionMode.current) { - painterResource(DesignsystemR.drawable.ic_placeholder_default) - } else { - // TODO b/228077205, show specific loading image visual - null - }, + + SubcomposeAsyncImage( modifier = Modifier .fillMaxWidth() .height(180.dp), contentScale = ContentScale.Crop, model = headerImageUrl, // TODO b/226661685: Investigate using alt text of image to populate content description - contentDescription = null, // decorative image + contentDescription = null, // decorative image, + error = { + if (LocalInspectionMode.current) { + Image( + painter = + painterResource(DesignsystemR.drawable.ic_placeholder_default), + contentDescription = "placeholder image", + ) + } else { + null + } + }, + loading = { + Box( + modifier = Modifier.size(180.dp), + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator( + Modifier.size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + }, ) } From 3284eb68a3b7ef2a4e3411e3b868dfcdef42368a Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Sat, 22 Jul 2023 12:20:49 +0300 Subject: [PATCH 02/13] feat: Show loading progress for Image Component as a solve for //TODO b/228077205, --- .../component/DynamicAsyncImage.kt | 33 ++-------------- .../nowinandroid/core/ui/NewsResourceCard.kt | 38 ++++--------------- 2 files changed, 11 insertions(+), 60 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index abd783545..e26a824af 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -16,19 +16,11 @@ package com.google.samples.apps.nowinandroid.core.designsystem.component -import androidx.compose.foundation.Image -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.size -import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable -import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.painter.Painter -import androidx.compose.ui.unit.dp import coil.compose.AsyncImage -import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** @@ -42,30 +34,11 @@ fun DynamicAsyncImage( placeholder: Painter? = null, ) { val iconTint = LocalTintTheme.current.iconTint - SubcomposeAsyncImage( - error = { - if (placeholder != null) { - Image( - painter = placeholder, - contentDescription = "placeholder image", - ) - } - }, + AsyncImage( + placeholder = placeholder, model = imageUrl, contentDescription = contentDescription, colorFilter = if (iconTint != null) ColorFilter.tint(iconTint) else null, modifier = modifier, - loading = { - Box( - modifier = modifier, - contentAlignment = Alignment.Center, - ) { - CircularProgressIndicator( - Modifier.size(80.dp), - color = MaterialTheme.colorScheme.tertiary, - ) - } - }, - - ) + ) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 0342bfcaf..c9a327881 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -17,7 +17,6 @@ package com.google.samples.apps.nowinandroid.core.ui import androidx.compose.foundation.Canvas -import androidx.compose.foundation.Image import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -32,7 +31,6 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme @@ -46,7 +44,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment -import androidx.compose.ui.Alignment.Companion import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale @@ -61,8 +58,6 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import coil.compose.AsyncImage -import coil.compose.AsyncImagePainter -import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -152,37 +147,20 @@ fun NewsResourceCardExpanded( fun NewsResourceHeaderImage( headerImageUrl: String?, ) { - - SubcomposeAsyncImage( + AsyncImage( + placeholder = if (LocalInspectionMode.current) { + painterResource(DesignsystemR.drawable.ic_placeholder_default) + } else { + // TODO b/228077205, show specific loading image visual + null + }, modifier = Modifier .fillMaxWidth() .height(180.dp), contentScale = ContentScale.Crop, model = headerImageUrl, // TODO b/226661685: Investigate using alt text of image to populate content description - contentDescription = null, // decorative image, - error = { - if (LocalInspectionMode.current) { - Image( - painter = - painterResource(DesignsystemR.drawable.ic_placeholder_default), - contentDescription = "placeholder image", - ) - } else { - null - } - }, - loading = { - Box( - modifier = Modifier.size(180.dp), - contentAlignment = Alignment.Center, - ) { - CircularProgressIndicator( - Modifier.size(80.dp), - color = MaterialTheme.colorScheme.tertiary, - ) - } - }, + contentDescription = null, // decorative image ) } From 7a05f7db96b647f009795059c4ed63eab11fc9e7 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Sat, 22 Jul 2023 12:37:44 +0300 Subject: [PATCH 03/13] feat: Show loading progress for Image Component as a solve for //TODO b/228077205, --- .../component/DynamicAsyncImage.kt | 33 +++++++++++-- .../nowinandroid/core/ui/NewsResourceCard.kt | 46 +++++++++++++------ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index e26a824af..abd783545 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -16,11 +16,19 @@ package com.google.samples.apps.nowinandroid.core.designsystem.component +import androidx.compose.foundation.Image +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.painter.Painter +import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** @@ -34,11 +42,30 @@ fun DynamicAsyncImage( placeholder: Painter? = null, ) { val iconTint = LocalTintTheme.current.iconTint - AsyncImage( - placeholder = placeholder, + SubcomposeAsyncImage( + error = { + if (placeholder != null) { + Image( + painter = placeholder, + contentDescription = "placeholder image", + ) + } + }, model = imageUrl, contentDescription = contentDescription, colorFilter = if (iconTint != null) ColorFilter.tint(iconTint) else null, modifier = modifier, - ) + loading = { + Box( + modifier = modifier, + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator( + Modifier.size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + }, + + ) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index c9a327881..aed1871fa 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -17,6 +17,7 @@ package com.google.samples.apps.nowinandroid.core.ui import androidx.compose.foundation.Canvas +import androidx.compose.foundation.Image import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -31,6 +32,7 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme @@ -44,6 +46,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment +import androidx.compose.ui.Alignment.Companion import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale @@ -58,6 +61,8 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.AsyncImagePainter +import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -70,7 +75,6 @@ import kotlinx.datetime.Instant import kotlinx.datetime.toJavaInstant import java.time.ZoneId import java.time.format.DateTimeFormatter -import java.time.format.FormatStyle import java.util.Locale import com.google.samples.apps.nowinandroid.core.designsystem.R as DesignsystemR @@ -147,20 +151,37 @@ fun NewsResourceCardExpanded( fun NewsResourceHeaderImage( headerImageUrl: String?, ) { - AsyncImage( - placeholder = if (LocalInspectionMode.current) { - painterResource(DesignsystemR.drawable.ic_placeholder_default) - } else { - // TODO b/228077205, show specific loading image visual - null - }, + + SubcomposeAsyncImage( modifier = Modifier .fillMaxWidth() .height(180.dp), contentScale = ContentScale.Crop, model = headerImageUrl, // TODO b/226661685: Investigate using alt text of image to populate content description - contentDescription = null, // decorative image + contentDescription = null, // decorative image, + error = { + if (LocalInspectionMode.current) { + Image( + painter = + painterResource(DesignsystemR.drawable.ic_placeholder_default), + contentDescription = "placeholder image", + ) + } else { + null + } + }, + loading = { + Box( + modifier = Modifier.size(180.dp), + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator( + Modifier.size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + }, ) } @@ -231,11 +252,8 @@ fun dateFormatted(publishDate: Instant): String { } } - return DateTimeFormatter - .ofLocalizedDate(FormatStyle.MEDIUM) - .withLocale(Locale.getDefault()) - .withZone(zoneId) - .format(publishDate.toJavaInstant()) + return DateTimeFormatter.ofPattern("MMM d, yyyy") + .withZone(zoneId).format(publishDate.toJavaInstant()) } @Composable From a294adb6b9bced0ec9db1534e7ca60215d8859b6 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Tue, 25 Jul 2023 01:22:21 +0300 Subject: [PATCH 04/13] feat: fix format issue --- .../core/designsystem/component/DynamicAsyncImage.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index abd783545..97c1252ce 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -66,6 +66,5 @@ fun DynamicAsyncImage( ) } }, - - ) + ) } From 8a197800c5924be812865a6af44a9957ee39a194 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Tue, 25 Jul 2023 08:28:08 +0300 Subject: [PATCH 05/13] feat: fix format issue --- .../samples/apps/nowinandroid/core/ui/NewsResourceCard.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index aed1871fa..9c8adaa59 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -46,7 +46,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment -import androidx.compose.ui.Alignment.Companion import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale @@ -60,8 +59,6 @@ import androidx.compose.ui.semantics.semantics import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp -import coil.compose.AsyncImage -import coil.compose.AsyncImagePainter import coil.compose.SubcomposeAsyncImage import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag @@ -151,7 +148,6 @@ fun NewsResourceCardExpanded( fun NewsResourceHeaderImage( headerImageUrl: String?, ) { - SubcomposeAsyncImage( modifier = Modifier .fillMaxWidth() From 57025e75874625efcf80c6cc40bd0bd5c88f8736 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Wed, 26 Jul 2023 08:40:24 +0300 Subject: [PATCH 06/13] fix: review notes --- .../apps/nowinandroid/core/ui/NewsResourceCard.kt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 9c8adaa59..50f41d8a1 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -72,6 +72,7 @@ import kotlinx.datetime.Instant import kotlinx.datetime.toJavaInstant import java.time.ZoneId import java.time.format.DateTimeFormatter +import java.time.format.FormatStyle import java.util.Locale import com.google.samples.apps.nowinandroid.core.designsystem.R as DesignsystemR @@ -157,15 +158,11 @@ fun NewsResourceHeaderImage( // TODO b/226661685: Investigate using alt text of image to populate content description contentDescription = null, // decorative image, error = { - if (LocalInspectionMode.current) { Image( painter = painterResource(DesignsystemR.drawable.ic_placeholder_default), contentDescription = "placeholder image", ) - } else { - null - } }, loading = { Box( @@ -248,8 +245,12 @@ fun dateFormatted(publishDate: Instant): String { } } - return DateTimeFormatter.ofPattern("MMM d, yyyy") - .withZone(zoneId).format(publishDate.toJavaInstant()) + return DateTimeFormatter + .ofLocalizedDate(FormatStyle.MEDIUM) + .withLocale(Locale.getDefault()) + .withZone(zoneId) + .format(publishDate.toJavaInstant()) + } @Composable From 9c1ec17a39103f27c642440132bd0414f9f00e37 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Thu, 27 Jul 2023 21:53:34 +0300 Subject: [PATCH 07/13] fix: modifier to Modifier --- .../core/designsystem/component/DynamicAsyncImage.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index 97c1252ce..ba5a890fe 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -57,7 +57,7 @@ fun DynamicAsyncImage( modifier = modifier, loading = { Box( - modifier = modifier, + modifier = Modifier, contentAlignment = Alignment.Center, ) { CircularProgressIndicator( From f1a2993adc8aa3e085ae29604a8976e32a462074 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Thu, 27 Jul 2023 22:03:12 +0300 Subject: [PATCH 08/13] feat: remove add placeholder todo --- .../samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index eaa0c58fa..145a7dbbc 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -394,7 +394,6 @@ fun TopicIcon( modifier: Modifier = Modifier, ) { DynamicAsyncImage( - // TODO b/228077205, show loading image visual instead of static placeholder placeholder = painterResource(R.drawable.ic_icon_placeholder), imageUrl = imageUrl, contentDescription = null, // decorative From e2762125740a9bfba5fece69ef48b19bc7473018 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Thu, 27 Jul 2023 22:04:26 +0300 Subject: [PATCH 09/13] feat: remove check if placeholder null and put a default placeholder if its not passed. --- .../core/designsystem/component/DynamicAsyncImage.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index ba5a890fe..94e65a780 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -26,9 +26,11 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.painter.Painter +import androidx.compose.ui.res.painterResource import androidx.compose.ui.unit.dp import coil.compose.AsyncImage import coil.compose.SubcomposeAsyncImage +import com.google.samples.apps.nowinandroid.core.designsystem.R import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** @@ -44,12 +46,10 @@ fun DynamicAsyncImage( val iconTint = LocalTintTheme.current.iconTint SubcomposeAsyncImage( error = { - if (placeholder != null) { - Image( - painter = placeholder, - contentDescription = "placeholder image", - ) - } + Image( + painter = placeholder ?: painterResource(R.drawable.ic_placeholder_default), + contentDescription = "placeholder image", + ) }, model = imageUrl, contentDescription = contentDescription, From 3595d8dcb509aad1e70ac5b0dfcbf793c18fada5 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Sat, 29 Jul 2023 12:13:49 +0300 Subject: [PATCH 10/13] feat: move the placeholder default value to the method --- .../core/designsystem/component/DynamicAsyncImage.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index 94e65a780..7a3c39393 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -41,13 +41,13 @@ fun DynamicAsyncImage( imageUrl: String, contentDescription: String?, modifier: Modifier = Modifier, - placeholder: Painter? = null, + placeholder: Painter = painterResource(R.drawable.ic_placeholder_default), ) { val iconTint = LocalTintTheme.current.iconTint SubcomposeAsyncImage( error = { Image( - painter = placeholder ?: painterResource(R.drawable.ic_placeholder_default), + painter = placeholder, contentDescription = "placeholder image", ) }, From 5bbcc904e9ac08bd3725c76e1fbfff396524a612 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Sat, 29 Jul 2023 12:16:18 +0300 Subject: [PATCH 11/13] feat: make painter in one line --- .../samples/apps/nowinandroid/core/ui/NewsResourceCard.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 50f41d8a1..621bf143c 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -159,8 +159,7 @@ fun NewsResourceHeaderImage( contentDescription = null, // decorative image, error = { Image( - painter = - painterResource(DesignsystemR.drawable.ic_placeholder_default), + painter = painterResource(DesignsystemR.drawable.ic_placeholder_default), contentDescription = "placeholder image", ) }, From f203e9a2185e3171cc3fd9e2d36babd220910425 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Thu, 10 Aug 2023 21:35:30 +0300 Subject: [PATCH 12/13] feat: replacing SubcomposeAsyncImage as its not recommended to use with lazyLayout --- .../component/DynamicAsyncImage.kt | 58 +++++++++++------ .../nowinandroid/core/ui/NewsResourceCard.kt | 65 +++++++++++-------- 2 files changed, 77 insertions(+), 46 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index 7a3c39393..4a5632078 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -18,19 +18,30 @@ package com.google.samples.apps.nowinandroid.core.designsystem.component import androidx.compose.foundation.Image import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.size import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.painter.Painter +import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.res.painterResource import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.AsyncImagePainter.State.Error +import coil.compose.AsyncImagePainter.State.Loading import coil.compose.SubcomposeAsyncImage +import coil.compose.rememberAsyncImagePainter import com.google.samples.apps.nowinandroid.core.designsystem.R +import com.google.samples.apps.nowinandroid.core.designsystem.R.drawable import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** @@ -44,27 +55,34 @@ fun DynamicAsyncImage( placeholder: Painter = painterResource(R.drawable.ic_placeholder_default), ) { val iconTint = LocalTintTheme.current.iconTint - SubcomposeAsyncImage( - error = { - Image( - painter = placeholder, - contentDescription = "placeholder image", - ) - }, + var isLoading by remember { mutableStateOf(true) } + var isError by remember { mutableStateOf(false) } + val imageLoader = rememberAsyncImagePainter( model = imageUrl, - contentDescription = contentDescription, - colorFilter = if (iconTint != null) ColorFilter.tint(iconTint) else null, - modifier = modifier, - loading = { - Box( - modifier = Modifier, - contentAlignment = Alignment.Center, - ) { - CircularProgressIndicator( - Modifier.size(80.dp), - color = MaterialTheme.colorScheme.tertiary, - ) - } + onState = { state -> + isLoading = state is Loading + isError = state is Error }, ) + Box( + modifier = modifier, + contentAlignment = Alignment.Center, + ) { + if (isLoading) { + // Display a progress bar while loading + CircularProgressIndicator( + modifier = Modifier + .align(Alignment.Center) + .size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + Image( + contentScale = ContentScale.Crop, + painter = if (isError.not()) imageLoader else placeholder, + contentDescription = contentDescription, + colorFilter = if (iconTint != null) ColorFilter.tint(iconTint) else null, + modifier = modifier, + ) + } } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 621bf143c..1fb8d41e5 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -44,6 +44,7 @@ import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -59,7 +60,12 @@ import androidx.compose.ui.semantics.semantics import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp -import coil.compose.SubcomposeAsyncImage +import coil.compose.AsyncImage +import coil.compose.AsyncImagePainter +import coil.compose.rememberAsyncImagePainter +import coil.compose.rememberImagePainter +import coil.request.ImageRequest +import com.google.samples.apps.nowinandroid.core.designsystem.R.drawable import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -74,7 +80,6 @@ import java.time.ZoneId import java.time.format.DateTimeFormatter import java.time.format.FormatStyle import java.util.Locale -import com.google.samples.apps.nowinandroid.core.designsystem.R as DesignsystemR /** * [NewsResource] card used on the following screens: For You, Saved @@ -149,32 +154,41 @@ fun NewsResourceCardExpanded( fun NewsResourceHeaderImage( headerImageUrl: String?, ) { - SubcomposeAsyncImage( - modifier = Modifier - .fillMaxWidth() - .height(180.dp), - contentScale = ContentScale.Crop, + var isLoading by remember { mutableStateOf(true) } + var isError by remember { mutableStateOf(false) } + val imageLoader = rememberAsyncImagePainter( model = headerImageUrl, - // TODO b/226661685: Investigate using alt text of image to populate content description - contentDescription = null, // decorative image, - error = { - Image( - painter = painterResource(DesignsystemR.drawable.ic_placeholder_default), - contentDescription = "placeholder image", - ) - }, - loading = { - Box( - modifier = Modifier.size(180.dp), - contentAlignment = Alignment.Center, - ) { - CircularProgressIndicator( - Modifier.size(80.dp), - color = MaterialTheme.colorScheme.tertiary, - ) - } + onState = { state -> + isLoading = state is AsyncImagePainter.State.Loading + isError = state is AsyncImagePainter.State.Error }, ) + Box( + modifier = Modifier + .fillMaxWidth() + .height(180.dp), + contentAlignment = Alignment.Center, + ) { + if (isLoading) { + // Display a progress bar while loading + CircularProgressIndicator( + modifier = Modifier + .align(Alignment.Center) + .size(80.dp), + color = MaterialTheme.colorScheme.tertiary, + ) + } + + Image( + modifier = Modifier + .fillMaxWidth() + .height(180.dp), + contentScale = ContentScale.Crop, + painter = if (isError.not()) imageLoader else painterResource(drawable.ic_placeholder_default), + // TODO b/226661685: Investigate using alt text of image to populate content description + contentDescription = null, // decorative image, + ) + } } @Composable @@ -249,7 +263,6 @@ fun dateFormatted(publishDate: Instant): String { .withLocale(Locale.getDefault()) .withZone(zoneId) .format(publishDate.toJavaInstant()) - } @Composable From 5d7e33d9051b0be66c91518ec50e2666def72491 Mon Sep 17 00:00:00 2001 From: qamarelsafadi Date: Fri, 11 Aug 2023 17:05:53 +0300 Subject: [PATCH 13/13] fix formatting --- .../core/designsystem/component/DynamicAsyncImage.kt | 4 ---- .../samples/apps/nowinandroid/core/ui/NewsResourceCard.kt | 4 ---- 2 files changed, 8 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt index 4a5632078..3bd3471ad 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/DynamicAsyncImage.kt @@ -18,8 +18,6 @@ package com.google.samples.apps.nowinandroid.core.designsystem.component import androidx.compose.foundation.Image import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.size import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme @@ -38,10 +36,8 @@ import androidx.compose.ui.unit.dp import coil.compose.AsyncImage import coil.compose.AsyncImagePainter.State.Error import coil.compose.AsyncImagePainter.State.Loading -import coil.compose.SubcomposeAsyncImage import coil.compose.rememberAsyncImagePainter import com.google.samples.apps.nowinandroid.core.designsystem.R -import com.google.samples.apps.nowinandroid.core.designsystem.R.drawable import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme /** diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 1fb8d41e5..4ed583b66 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -44,7 +44,6 @@ import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -60,11 +59,8 @@ import androidx.compose.ui.semantics.semantics import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp -import coil.compose.AsyncImage import coil.compose.AsyncImagePainter import coil.compose.rememberAsyncImagePainter -import coil.compose.rememberImagePainter -import coil.request.ImageRequest import com.google.samples.apps.nowinandroid.core.designsystem.R.drawable import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopicTag