QuickSettings back press addressing comments (#130)

* Fix quick settings back press * Update TestTags.kt * Fix quick settings back press * Update NavigationTest.kt
google · Mar 13, 2024 · 0889a45 · 0889a45
1 parent 01622d3
commit 0889a45
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 55 deletions.
diff --git a/.idea/androidTestResultsUserPreferences.xml b/.idea/androidTestResultsUserPreferences.xml
diff --git a/app/src/androidTest/java/com/google/jetpackcamera/BackgroundDeviceTest.kt b/app/src/androidTest/java/com/google/jetpackcamera/BackgroundDeviceTest.kt
@@ -21,10 +21,10 @@ import androidx.test.platform.app.InstrumentationRegistry
 import androidx.test.rule.GrantPermissionRule
 import androidx.test.uiautomator.By
 import androidx.test.uiautomator.UiDevice
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_BUTTON
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_RATIO_1_1_BUTTON
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_RATIO_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_DROP_DOWN
 import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_FLIP_CAMERA_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_RATIO_1_1_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_RATIO_BUTTON
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -61,19 +61,19 @@ class BackgroundDeviceTest {
 
     @Test
     fun flipCamera_then_background_foreground() {
-        uiDevice.findObject(By.res(QUICK_SETTINGS_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         uiDevice.findObject(By.res(QUICK_SETTINGS_FLIP_CAMERA_BUTTON)).click()
-        uiDevice.findObject(By.res(QUICK_SETTINGS_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         uiDevice.waitForIdle(2000)
         backgroundThenForegroundApp()
     }
 
     @Test
     fun setAspectRatio_then_background_foreground() {
-        uiDevice.findObject(By.res(QUICK_SETTINGS_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         uiDevice.findObject(By.res(QUICK_SETTINGS_RATIO_BUTTON)).click()
         uiDevice.findObject(By.res(QUICK_SETTINGS_RATIO_1_1_BUTTON)).click()
-        uiDevice.findObject(By.res(QUICK_SETTINGS_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         uiDevice.waitForIdle(2000)
         backgroundThenForegroundApp()
     }

diff --git a/app/src/androidTest/java/com/google/jetpackcamera/FlashDeviceTest.kt b/app/src/androidTest/java/com/google/jetpackcamera/FlashDeviceTest.kt
@@ -22,6 +22,9 @@ import androidx.test.rule.GrantPermissionRule
 import androidx.test.uiautomator.By
 import androidx.test.uiautomator.UiDevice
 import androidx.test.uiautomator.Until
+import com.google.jetpackcamera.feature.preview.ui.CAPTURE_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_DROP_DOWN
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_FLASH_BUTTON
 import com.google.jetpackcamera.settings.model.FlashMode
 import kotlinx.coroutines.test.runTest
 import org.junit.Before
@@ -48,9 +51,9 @@ internal class FlashDeviceTest {
     @Test
     fun set_flash_on() = runTest {
         uiDevice.waitForIdle()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         assert(
             UiTestUtil.getPreviewCameraAppSettings(activityScenario!!).flashMode ==
                 FlashMode.ON
@@ -60,10 +63,10 @@ internal class FlashDeviceTest {
     @Test
     fun set_flash_auto() = runTest {
         uiDevice.waitForIdle()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         assert(
             UiTestUtil.getPreviewCameraAppSettings(activityScenario!!).flashMode ==
                 FlashMode.AUTO
@@ -77,11 +80,11 @@ internal class FlashDeviceTest {
             UiTestUtil.getPreviewCameraAppSettings(activityScenario!!).flashMode ==
                 FlashMode.OFF
         )
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         assert(
             UiTestUtil.getPreviewCameraAppSettings(activityScenario!!).flashMode ==
                 FlashMode.OFF
@@ -91,11 +94,11 @@ internal class FlashDeviceTest {
     @Test
     fun set_screen_flash_and_capture_successfully() = runTest {
         uiDevice.waitForIdle()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         // flash on with front camera will automatically enable screen flash
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
-        uiDevice.findObject(By.res("CaptureButton")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
+        uiDevice.findObject(By.res(CAPTURE_BUTTON)).click()
         uiDevice.wait(
             Until.findObject(By.res("ImageCaptureSuccessToast")),
             5000
@@ -105,11 +108,11 @@ internal class FlashDeviceTest {
     @Test
     fun set_screen_flash_and_capture_with_screen_change_overlay_shown() = runTest {
         uiDevice.waitForIdle()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
         // flash on with front camera will automatically enable screen flash
-        uiDevice.findObject(By.res("QuickSetFlash")).click()
-        uiDevice.findObject(By.res("QuickSettingDropDown")).click()
-        uiDevice.findObject(By.res("CaptureButton")).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_FLASH_BUTTON)).click()
+        uiDevice.findObject(By.res(QUICK_SETTINGS_DROP_DOWN)).click()
+        uiDevice.findObject(By.res(CAPTURE_BUTTON)).click()
         uiDevice.wait(
             Until.findObject(By.res("ScreenFlashOverlay")),
             5000

diff --git a/app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt b/app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt
@@ -26,10 +26,10 @@ import androidx.test.rule.GrantPermissionRule
 import androidx.test.uiautomator.UiDevice
 import com.google.jetpackcamera.feature.preview.ui.CAPTURE_BUTTON
 import com.google.jetpackcamera.feature.preview.ui.FLIP_CAMERA_BUTTON
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_BUTTON
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_RATIO_1_1_BUTTON
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_RATIO_BUTTON
 import com.google.jetpackcamera.feature.preview.ui.SETTINGS_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_DROP_DOWN
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_RATIO_1_1_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_RATIO_BUTTON
 import com.google.jetpackcamera.settings.ui.BACK_BUTTON
 import org.junit.Rule
 import org.junit.Test
@@ -113,7 +113,7 @@ class NavigationTest {
         }
 
         // Navigate to the quick settings screen
-        composeTestRule.onNodeWithTag(QUICK_SETTINGS_BUTTON)
+        composeTestRule.onNodeWithTag(QUICK_SETTINGS_DROP_DOWN)
             .assertExists()
             .performClick()
 
@@ -125,8 +125,8 @@ class NavigationTest {
         // Press the device's back button
         uiDevice.pressBack()
 
-        // Assert we're on PreviewScreen by finding the capture button
-        composeTestRule.onNodeWithTag(CAPTURE_BUTTON).assertExists()
+        // Assert we're on PreviewScreen by finding the flip camera button
+        composeTestRule.onNodeWithTag(FLIP_CAMERA_BUTTON).assertExists()
     }
 
     @Test
@@ -137,7 +137,7 @@ class NavigationTest {
         }
 
         // Navigate to the quick settings screen
-        composeTestRule.onNodeWithTag(QUICK_SETTINGS_BUTTON)
+        composeTestRule.onNodeWithTag(QUICK_SETTINGS_DROP_DOWN)
             .assertExists()
             .performClick()
 

diff --git a/app/src/androidTest/java/com/google/jetpackcamera/SwitchCameraTest.kt b/app/src/androidTest/java/com/google/jetpackcamera/SwitchCameraTest.kt
@@ -30,7 +30,7 @@ import androidx.test.rule.GrantPermissionRule
 import com.google.common.truth.Truth.assertThat
 import com.google.jetpackcamera.feature.preview.ui.FLIP_CAMERA_BUTTON
 import com.google.jetpackcamera.feature.preview.ui.PREVIEW_DISPLAY
-import com.google.jetpackcamera.feature.preview.ui.QUICK_SETTINGS_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_DROP_DOWN
 import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_FLIP_CAMERA_BUTTON
 import com.google.jetpackcamera.quicksettings.R.string.quick_settings_back_camera_description
 import com.google.jetpackcamera.quicksettings.R.string.quick_settings_dropdown_closed_description
@@ -107,7 +107,7 @@ class SwitchCameraTest {
     @Test
     fun canFlipCamera_fromQuickSettings() = runFlipCameraTest(composeTestRule) {
         // Navigate to quick settings
-        composeTestRule.onNodeWithTag(QUICK_SETTINGS_BUTTON)
+        composeTestRule.onNodeWithTag(QUICK_SETTINGS_DROP_DOWN)
             .assertExists()
             .performClick()
 

diff --git a/benchmark/src/main/java/com/google/jetpackcamera/benchmark/Utils.kt b/benchmark/src/main/java/com/google/jetpackcamera/benchmark/Utils.kt
@@ -26,9 +26,9 @@ const val DEFAULT_TEST_ITERATIONS = 5
 
 // test tags
 const val CAPTURE_BUTTON = "CaptureButton"
-const val QUICK_SETTINGS_DROP_DOWN_BUTTON = "QuickSettingDropDown"
-const val QUICK_SETTINGS_FLASH_BUTTON = "QuickSetFlash"
-const val QUICK_SETTINGS_FLIP_CAMERA_BUTTON = "QuickSetFlipCamera"
+const val QUICK_SETTINGS_DROP_DOWN_BUTTON = "QuickSettingsDropDown"
+const val QUICK_SETTINGS_FLASH_BUTTON = "QuickSettingsFlashButton"
+const val QUICK_SETTINGS_FLIP_CAMERA_BUTTON = "QuickSettingsFlipCameraButton"
 const val IMAGE_CAPTURE_SUCCESS_TOAST = "ImageCaptureSuccessToast"
 
 // test descriptions

diff --git a/...review/src/main/java/com/google/jetpackcamera/feature/preview/ui/CameraControlsOverlay.kt b/...review/src/main/java/com/google/jetpackcamera/feature/preview/ui/CameraControlsOverlay.kt
@@ -200,7 +200,7 @@ private fun ControlsBottom(
             Row(Modifier.weight(1f), horizontalArrangement = Arrangement.SpaceEvenly) {
                 if (!isQuickSettingsOpen && videoRecordingState == VideoRecordingState.INACTIVE) {
                     FlipCameraButton(
-                        modifier = Modifier.testTag(FLIP_CAMERA_BUTTON),
+                        modifier = modifier.testTag(FLIP_CAMERA_BUTTON),
                         onClick = onFlipCamera,
                         // enable only when phone has front and rear camera
                         enabledCondition = currentCameraSettings.isBackCameraAvailable &&

diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ui/TestTags.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ui/TestTags.kt
@@ -19,6 +19,3 @@ const val CAPTURE_BUTTON = "CaptureButton"
 const val SETTINGS_BUTTON = "SettingsButton"
 const val PREVIEW_DISPLAY = "PreviewDisplay"
 const val FLIP_CAMERA_BUTTON = "FlipCameraButton"
-const val QUICK_SETTINGS_BUTTON = "QuickSettingDropDown"
-const val QUICK_SETTINGS_RATIO_BUTTON = "QuickSetAspectRatio"
-const val QUICK_SETTINGS_RATIO_1_1_BUTTON = "QuickSetAspectRatio1:1"
diff --git a/...tings/src/main/java/com/google/jetpackcamera/feature/quicksettings/QuickSettingsScreen.kt b/...tings/src/main/java/com/google/jetpackcamera/feature/quicksettings/QuickSettingsScreen.kt
@@ -37,7 +37,9 @@ import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.res.dimensionResource
 import com.google.jetpackcamera.feature.quicksettings.ui.ExpandedQuickSetRatio
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_FLASH_BUTTON
 import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_FLIP_CAMERA_BUTTON
+import com.google.jetpackcamera.feature.quicksettings.ui.QUICK_SETTINGS_RATIO_BUTTON
 import com.google.jetpackcamera.feature.quicksettings.ui.QuickFlipCamera
 import com.google.jetpackcamera.feature.quicksettings.ui.QuickSetCaptureMode
 import com.google.jetpackcamera.feature.quicksettings.ui.QuickSetFlash
@@ -95,14 +97,7 @@ fun QuickSettingsScreenOverlay(
                 .fillMaxSize()
                 .background(color = backgroundColor.value)
                 .alpha(alpha = contentAlpha.value)
-                .clickable {
-                    // if a setting is expanded, click on the background to close it.
-                    // if no other settings are expanded, then close the popup
-                    when (shouldShowQuickSetting) {
-                        IsExpandedQuickSetting.NONE -> toggleIsOpen()
-                        else -> shouldShowQuickSetting = IsExpandedQuickSetting.NONE
-                    }
-                },
+                .clickable(onClick = onBack),
             verticalArrangement = Arrangement.Center,
             horizontalAlignment = Alignment.CenterHorizontally
         ) {
@@ -159,7 +154,7 @@ private fun ExpandedQuickSettingsUi(
                     arrayOf(
                         {
                             QuickSetFlash(
-                                modifier = Modifier.testTag("QuickSetFlash"),
+                                modifier = Modifier.testTag(QUICK_SETTINGS_FLASH_BUTTON),
                                 onClick = { f: FlashMode -> onFlashModeClick(f) },
                                 currentFlashMode = currentCameraSettings.flashMode
                             )
@@ -173,7 +168,7 @@ private fun ExpandedQuickSettingsUi(
                         },
                         {
                             QuickSetRatio(
-                                modifier = Modifier.testTag("QuickSetAspectRatio"),
+                                modifier = Modifier.testTag(QUICK_SETTINGS_RATIO_BUTTON),
                                 onClick = {
                                     setVisibleQuickSetting(
                                         IsExpandedQuickSetting.ASPECT_RATIO

diff --git a/...rc/main/java/com/google/jetpackcamera/feature/quicksettings/ui/QuickSettingsComponents.kt b/...rc/main/java/com/google/jetpackcamera/feature/quicksettings/ui/QuickSettingsComponents.kt
@@ -78,7 +78,7 @@ fun ExpandedQuickSetRatio(setRatio: (aspectRatio: AspectRatio) -> Unit, currentR
             },
             {
                 QuickSetRatio(
-                    modifier = Modifier.testTag("QuickSetAspectRatio1:1"),
+                    modifier = Modifier.testTag(QUICK_SETTINGS_RATIO_1_1_BUTTON),
                     onClick = { setRatio(AspectRatio.ONE_ONE) },
                     ratio = AspectRatio.ONE_ONE,
                     currentRatio = currentRatio,
@@ -201,7 +201,7 @@ fun ToggleQuickSettingsButton(toggleDropDown: () -> Unit, isOpen: Boolean) {
                 stringResource(R.string.quick_settings_dropdown_closed_description)
             },
             modifier = Modifier
-                .testTag("QuickSettingDropDown")
+                .testTag(QUICK_SETTINGS_DROP_DOWN)
                 .size(72.dp)
                 .clickable {
                     toggleDropDown()

diff --git a/...quicksettings/src/main/java/com/google/jetpackcamera/feature/quicksettings/ui/TestTags.kt b/...quicksettings/src/main/java/com/google/jetpackcamera/feature/quicksettings/ui/TestTags.kt
@@ -15,4 +15,8 @@
  */
 package com.google.jetpackcamera.feature.quicksettings.ui
 
+const val QUICK_SETTINGS_DROP_DOWN = "QuickSettingsDropDown"
 const val QUICK_SETTINGS_FLIP_CAMERA_BUTTON = "QuickSettingsFlipCameraButton"
+const val QUICK_SETTINGS_RATIO_BUTTON = "QuickSettingsRatioButton"
+const val QUICK_SETTINGS_FLASH_BUTTON = "QuickSettingsFlashButton"
+const val QUICK_SETTINGS_RATIO_1_1_BUTTON = "QuickSettingsAspectRatio1:1Button"