From 31d6ad02f16a8b619808583fbfae9c31de91b2b6 Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Thu, 11 Apr 2024 22:22:02 +0900 Subject: [PATCH 1/8] Remove onNamePosition. --- .../compose/plantdetail/PlantDetailView.kt | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt index 0d8b12f24..4c3dc4b47 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt @@ -66,8 +66,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale -import androidx.compose.ui.layout.onGloballyPositioned -import androidx.compose.ui.layout.positionInWindow import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.pluralStringResource @@ -179,14 +177,6 @@ fun PlantDetails( PlantDetailsContent( scrollState = scrollState, toolbarState = toolbarState, - onNamePosition = { newNamePosition -> - // Comparing to Float.MIN_VALUE as we are just interested on the original - // position of name on the screen - if (plantScroller.namePosition == Float.MIN_VALUE) { - plantScroller = - plantScroller.copy(namePosition = newNamePosition) - } - }, plant = plant, isPlanted = isPlanted, hasValidUnsplashKey = hasValidUnsplashKey, @@ -217,7 +207,6 @@ private fun PlantDetailsContent( isPlanted: Boolean, hasValidUnsplashKey: Boolean, imageHeight: Dp, - onNamePosition: (Float) -> Unit, onFabClick: () -> Unit, onGalleryClick: () -> Unit, contentAlpha: () -> Float, @@ -255,7 +244,6 @@ private fun PlantDetailsContent( wateringInterval = plant.wateringInterval, description = plant.description, hasValidUnsplashKey = hasValidUnsplashKey, - onNamePosition = { onNamePosition(it) }, toolbarState = toolbarState, onGalleryClick = onGalleryClick, modifier = Modifier.constrainAs(info) { @@ -483,7 +471,6 @@ private fun PlantInformation( wateringInterval: Int, description: String, hasValidUnsplashKey: Boolean, - onNamePosition: (Float) -> Unit, toolbarState: ToolbarState, onGalleryClick: () -> Unit, modifier: Modifier = Modifier @@ -499,7 +486,6 @@ private fun PlantInformation( bottom = Dimens.PaddingNormal ) .align(Alignment.CenterHorizontally) - .onGloballyPositioned { onNamePosition(it.positionInWindow().y) } .visible { toolbarState == ToolbarState.HIDDEN } ) Box( From 491bdaff36bf364cf22cbc67fe497ae4ccaa793e Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Thu, 11 Apr 2024 22:30:54 +0900 Subject: [PATCH 2/8] Pass detailAppbarHeight parameter. --- .../apps/sunflower/compose/plantdetail/PlantDetailView.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt index 4c3dc4b47..d188073ab 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt @@ -151,10 +151,11 @@ fun PlantDetails( callbacks: PlantDetailsCallbacks, modifier: Modifier = Modifier ) { + val detailAppbarHeight = Dimens.PlantDetailAppBarHeight // PlantDetails owns the scrollerPosition to simulate CollapsingToolbarLayout's behavior val scrollState = rememberScrollState() - var plantScroller by remember { - mutableStateOf(PlantDetailsScroller(scrollState, Float.MIN_VALUE)) + val plantScroller by remember { + mutableStateOf(PlantDetailsScroller(scrollState, detailAppbarHeight)) } val transitionState = remember(plantScroller) { plantScroller.toolbarTransitionState } From 43ac644669173e9b995a08b18387147bcd778c27 Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Thu, 11 Apr 2024 22:31:32 +0900 Subject: [PATCH 3/8] Change namePosition to detailAppbarHeight. --- .../apps/sunflower/compose/plantdetail/PlantDetailScroller.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt index 1b139c3f7..e3c33e3c9 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt @@ -19,6 +19,7 @@ package com.google.samples.apps.sunflower.compose.plantdetail import androidx.compose.animation.core.MutableTransitionState import androidx.compose.foundation.ScrollState import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp // Value obtained empirically so that the header buttons don't surpass the header container @@ -29,7 +30,7 @@ private val HeaderTransitionOffset = 190.dp */ data class PlantDetailsScroller( val scrollState: ScrollState, - val namePosition: Float + val detailAppbarHeight: Dp, ) { val toolbarTransitionState = MutableTransitionState(ToolbarState.HIDDEN) From 91fac09ba8a4516596b1e9fcf7b636dfd293530b Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Thu, 11 Apr 2024 22:32:01 +0900 Subject: [PATCH 4/8] Calc transitionOffset from detailAppbarHeight. --- .../sunflower/compose/plantdetail/PlantDetailScroller.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt index e3c33e3c9..4aeb101e3 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt @@ -48,9 +48,10 @@ data class PlantDetailsScroller( } } - private fun getTransitionOffset(density: Density): Float = with(density) { - HeaderTransitionOffset.toPx() - } + private fun getTransitionOffset(density: Density, detailAppbarHeight: Dp): Float = + with(density) { + (detailAppbarHeight - HeaderTransitionOffset).toPx() + } } // Toolbar state related classes and functions to achieve the CollapsingToolbarLayout animation From 4bbf171b4cd236138c139efabe746878a802ac71 Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Thu, 11 Apr 2024 22:32:17 +0900 Subject: [PATCH 5/8] Check scrollState value. --- .../apps/sunflower/compose/plantdetail/PlantDetailScroller.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt index 4aeb101e3..52044e2fe 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailScroller.kt @@ -37,8 +37,8 @@ data class PlantDetailsScroller( fun getToolbarState(density: Density): ToolbarState { // When the namePosition is placed correctly on the screen (position > 1f) and it's // position is close to the header, then show the toolbar. - return if (namePosition > 1f && - scrollState.value > (namePosition - getTransitionOffset(density)) + return if (scrollState.value > 0 && + scrollState.value > getTransitionOffset(density, detailAppbarHeight) ) { toolbarTransitionState.targetState = ToolbarState.SHOWN ToolbarState.SHOWN From 3482e3c5a352fea6ac2c6c99bfe2d7ab0df6b1d6 Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Fri, 12 Apr 2024 16:56:57 +0900 Subject: [PATCH 6/8] Create test function - plantDetails_scrollDown_visibleToolbar. --- .../plantdetail/PlantDetailComposeTest.kt | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt b/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt index 999ea4551..941ed0c27 100644 --- a/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt @@ -22,13 +22,16 @@ import androidx.annotation.RawRes import androidx.compose.runtime.Composable import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotDisplayed import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot +import androidx.compose.ui.test.performTouchInput +import androidx.compose.ui.test.swipeUp import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.google.samples.apps.sunflower.compose.plantdetail.PlantDetails -import com.google.samples.apps.sunflower.compose.plantdetail.PlantDetailsCallbacks import com.google.samples.apps.sunflower.data.Plant import com.google.samples.apps.sunflower.test.R import org.junit.Rule @@ -67,6 +70,23 @@ class PlantDetailComposeTest { composeTestRule.onNodeWithContentDescription("Gallery Icon").assertIsDisplayed() } + @Test + fun plantDetails_scrollDown_visibleToolbar() { + startPlantDetails(isPlanted = false) + val headerAction = composeTestRule.onNodeWithTag("Header Action") + val detailToolbar = composeTestRule.onNodeWithTag("Detail Toolbar") + + headerAction.assertIsDisplayed() + detailToolbar.assertIsNotDisplayed() + + composeTestRule.onRoot().performTouchInput { + swipeUp() + } + + headerAction.assertIsNotDisplayed() + detailToolbar.assertIsDisplayed() + } + private fun startPlantDetails(isPlanted: Boolean, hasUnsplashKey: Boolean = false) { composeTestRule.setContent { PlantDetails( @@ -98,4 +118,4 @@ internal fun plantForTesting(): Plant { private fun rawUri(@RawRes id: Int): Uri { return "${ContentResolver.SCHEME_ANDROID_RESOURCE}://${LocalContext.current.packageName}/$id" .toUri() -} \ No newline at end of file +} From bcbdf6735eda5b081021fba7c810f3d2fefab70d Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Fri, 12 Apr 2024 16:58:04 +0900 Subject: [PATCH 7/8] Add testTag to test. --- .../sunflower/compose/plantdetail/PlantDetailView.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt index d188073ab..5db34b7f3 100644 --- a/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt +++ b/app/src/main/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailView.kt @@ -67,6 +67,7 @@ import androidx.compose.ui.draw.alpha import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource @@ -348,13 +349,17 @@ private fun PlantToolbar( plantName = plantName, onBackClick = callbacks.onBackClick, onShareClick = onShareClick, - modifier = Modifier.alpha(toolbarAlpha()) + modifier = Modifier + .alpha(toolbarAlpha()) + .testTag("Detail Toolbar") ) } else { PlantHeaderActions( onBackClick = callbacks.onBackClick, onShareClick = onShareClick, - modifier = Modifier.alpha(contentAlpha()) + modifier = Modifier + .alpha(contentAlpha()) + .testTag("Header Action") ) } } From 1a846027cf5901c3463abfc2faa4eb67036ab1a0 Mon Sep 17 00:00:00 2001 From: Jaehwa Noh Date: Fri, 12 Apr 2024 17:01:02 +0900 Subject: [PATCH 8/8] Add KDoc to PlantDetailComposeTest. --- .../sunflower/compose/plantdetail/PlantDetailComposeTest.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt b/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt index 941ed0c27..9af495ade 100644 --- a/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/sunflower/compose/plantdetail/PlantDetailComposeTest.kt @@ -38,6 +38,9 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +/** + * Instrumented test for [PlantDetails] + */ @RunWith(AndroidJUnit4::class) class PlantDetailComposeTest {