-
Notifications
You must be signed in to change notification settings - Fork 66
Update Capture Button visuals #461
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.
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
Open
temcguir
wants to merge
43
commits into
main
Choose a base branch
from
temcguir/update_capture_button_appearance
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
dfb50e4
Increase test timeouts for API 28 emulators
temcguir 3494bdf
Update tests to use common timeouts rather than `waitUntil` directly
temcguir 0090aed
Update style guide with some guidance on timeouts in tests
temcguir 0787e1a
Add isCameraRunning state
temcguir 0e9aa1b
Update more tests to use `waitForCaptureButton`
temcguir 74fd572
Move visually debounced button state into its own `remember` hook
temcguir c1db907
Rename CaptureButtonUiState.Enabled to "Available"
temcguir 4833dac
Add isEnabled property to CaptureButtonUiState to control button inte…
temcguir db5fdbf
Add documentation to CaptureButtonUiState
temcguir 728a5cf
Add new test for CaptureButtonUiStateAdapter
temcguir 1f5bccd
Add new Compose Previews for disabled capture button states
temcguir a4dea88
Remove repeated logic from `CaptureModeSettingsTest`
temcguir 781b62e
Merge branch 'temcguir/improve_test_flakiness' into temcguir/capture_…
temcguir e448e80
Update CaptureButton Compose Previews to use real logic
temcguir cc0ca6b
Simplify Preview logic for CaptureButtonComponent
temcguir 89b9ddb
Ensure pressed state for IMAGE_ONLY is right color
temcguir 4737c83
Make capture button nucles correct color and size during capture
temcguir 539a226
Ensure capture animation works with volume buttons
temcguir 828c02e
Apply Spotless
temcguir 940a090
Adjust size of capture button, ring stroke, and nucleus to match mocks
temcguir 098cc25
Make capture button nucleus white when in idle VIDEO_ONLY mode
temcguir 6e8f7c3
Add 50% black background to capture button to match mocks
temcguir 647522b
Update compose previews to use a gradient background for higher vis
temcguir 3553cd4
merge: resolve conflicts with main and keep PR 461 visuals
temcguir 9d3a498
Setup build environment and dependencies for testing and screenshots
temcguir 0c6620f
Improve accessibility of CaptureButton with content descriptions and …
temcguir b8e750a
Update CaptureButton visuals and animations to match spec
temcguir ef6f3ca
Implement accurate overlap detection for dynamic background
temcguir afe1a4e
Add screenshot tests and previews for all permutations
temcguir e3a84ad
Add generated reference screenshots for all permutations
temcguir 8b4eee5
Extract magic numbers to constants and update styleguide
temcguir cbfcbe9
Add tests for PressedRecording and Disabled states of CaptureButton
temcguir 08496f4
Enable automated accessibility checks and make CaptureButton focusable
temcguir 74fc3fc
Update styleguide with explicit focusability rule for custom components
temcguir 0d89719
Fix accessibility issues in CaptureButton:
temcguir 29c4a18
Fix ring visibility in previews in CaptureButtonComponents.kt
temcguir 27b6820
Consolidate previews and apply style guide fixes in CaptureButtonScre…
temcguir 409ef19
Refactor capture button previews to use CompositionLocal for pressed …
temcguir 837e4e2
Use string resources for CaptureButton content descriptions
temcguir 5ba312f
Address review comments: move alpha values and durations to constants…
temcguir b4e3938
Merge branch 'main' into temcguir/update_capture_button_appearance
temcguir 38f01e4
Clean up unnecessary changes and debug logs
temcguir 68cc381
Apply spotless formatting to CaptureLayout.kt
temcguir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,12 +19,15 @@ plugins { | |
| alias(libs.plugins.kotlin.android) | ||
| alias(libs.plugins.kotlin.kapt) | ||
| alias(libs.plugins.compose.compiler) | ||
| alias(libs.plugins.compose.screenshot) | ||
| } | ||
|
|
||
| android { | ||
| namespace = "com.google.jetpackcamera.ui.components.capture" | ||
| compileSdk = libs.versions.compileSdk.get().toInt() | ||
|
|
||
| experimentalProperties["android.experimental.enableScreenshotTest"] = true | ||
|
|
||
| defaultConfig { | ||
| minSdk = libs.versions.minSdk.get().toInt() | ||
| testOptions.targetSdk = libs.versions.targetSdk.get().toInt() | ||
|
|
@@ -87,6 +90,8 @@ dependencies { | |
| // noinspection TestManifestGradleConfiguration: required for release build unit tests | ||
| testImplementation(libs.compose.test.manifest) | ||
| testImplementation(libs.compose.junit) | ||
| screenshotTestImplementation(libs.screenshot.validation.api) | ||
| screenshotTestImplementation(libs.compose.ui.tooling) | ||
|
|
||
| // Testing | ||
| testImplementation(libs.junit) | ||
|
|
@@ -98,6 +103,9 @@ dependencies { | |
| implementation(libs.androidx.junit) | ||
| androidTestImplementation(libs.androidx.junit) | ||
| androidTestImplementation(libs.androidx.espresso.core) | ||
| androidTestImplementation(libs.androidx.espresso.accessibility) | ||
| androidTestImplementation(libs.compose.accessibility) | ||
| androidTestImplementation(libs.accessibility.test.framework) | ||
|
|
||
| implementation(project(":ui:uistate")) | ||
| implementation(project(":ui:uistate:capture")) | ||
|
|
@@ -116,3 +124,8 @@ dependencies { | |
| kapt { | ||
| correctErrorTypes = true | ||
| } | ||
| configurations.all { | ||
| resolutionStrategy { | ||
| exclude(group = "com.google.protobuf", module = "protobuf-lite") | ||
| } | ||
| } | ||
|
Comment on lines
+127
to
+131
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we include a comment why this is necessary |
||
189 changes: 189 additions & 0 deletions
189
.../src/androidTest/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| /* | ||
| * Copyright (C) 2026 The Android Open Source Project | ||
| * | ||
| * 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 | ||
| * | ||
| * http://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. | ||
| */ | ||
| package com.google.jetpackcamera.ui.components.capture | ||
|
|
||
| import androidx.activity.ComponentActivity | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.platform.testTag | ||
| import androidx.compose.ui.semantics.Role | ||
| import androidx.compose.ui.semantics.SemanticsProperties | ||
| import androidx.compose.ui.test.SemanticsMatcher | ||
| import androidx.compose.ui.test.assert | ||
| import androidx.compose.ui.test.assertContentDescriptionEquals | ||
| import androidx.compose.ui.test.isNotEnabled | ||
| import androidx.compose.ui.test.junit4.accessibility.enableAccessibilityChecks | ||
| import androidx.compose.ui.test.junit4.createAndroidComposeRule | ||
| import androidx.compose.ui.test.onNodeWithTag | ||
| import androidx.compose.ui.test.onRoot | ||
| import androidx.compose.ui.test.tryPerformAccessibilityChecks | ||
| import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
| import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResult.AccessibilityCheckResultType | ||
| import com.google.android.apps.common.testing.accessibility.framework.integrations.espresso.AccessibilityValidator | ||
| import com.google.jetpackcamera.model.CaptureMode | ||
| import com.google.jetpackcamera.ui.uistate.capture.CaptureButtonUiState | ||
| import org.junit.Before | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class CaptureButtonTest { | ||
| @get:Rule | ||
| val composeTestRule = createAndroidComposeRule<ComponentActivity>() | ||
|
|
||
| @Before | ||
| fun setUp() { | ||
| composeTestRule.enableAccessibilityChecks( | ||
| AccessibilityValidator().setRunChecksFromRootView(true).also { | ||
| it.setThrowExceptionFor(AccessibilityCheckResultType.ERROR) | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_standard_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonTestTag"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.STANDARD) | ||
| ) | ||
| } | ||
|
|
||
| composeTestRule.onNodeWithTag("CaptureButtonTestTag").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonTestTag" | ||
| ).assertContentDescriptionEquals("Capture Photo") | ||
| composeTestRule.onNodeWithTag("CaptureButtonTestTag", useUnmergedTree = true) | ||
| .assert(SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button)) | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_imageOnly_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonImageOnly"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.IMAGE_ONLY) | ||
| ) | ||
| } | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| composeTestRule.onNodeWithTag("CaptureButtonImageOnly").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonImageOnly" | ||
| ).assertContentDescriptionEquals("Capture Photo") | ||
| composeTestRule.onNodeWithTag("CaptureButtonImageOnly", useUnmergedTree = true) | ||
| .assert(SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_videoOnly_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonVideoOnly"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.VIDEO_ONLY) | ||
| ) | ||
| } | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| composeTestRule.onNodeWithTag("CaptureButtonVideoOnly").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonVideoOnly" | ||
| ).assertContentDescriptionEquals("Start Video Recording") | ||
| composeTestRule.onNodeWithTag("CaptureButtonVideoOnly", useUnmergedTree = true) | ||
| .assert(SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_lockedRecording_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonLocked"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Recording.LockedRecording | ||
| ) | ||
| } | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| composeTestRule.onNodeWithTag("CaptureButtonLocked").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonLocked" | ||
| ).assertContentDescriptionEquals("Stop Video Recording") | ||
| composeTestRule.onNodeWithTag("CaptureButtonLocked", useUnmergedTree = true) | ||
| .assert(SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_pressedRecording_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonPressedRecording"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Recording.PressedRecording | ||
| ) | ||
| } | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| composeTestRule.onNodeWithTag("CaptureButtonPressedRecording").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonPressedRecording" | ||
| ).assertContentDescriptionEquals("Recording Video") | ||
| composeTestRule.onNodeWithTag("CaptureButtonPressedRecording", useUnmergedTree = true) | ||
| .assert(SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureButton_disabled_exists() { | ||
| composeTestRule.setContent { | ||
| CaptureButton( | ||
| modifier = Modifier.testTag("CaptureButtonDisabled"), | ||
| onImageCapture = {}, | ||
| onStartRecording = {}, | ||
| onStopRecording = {}, | ||
| onLockVideoRecording = {}, | ||
| onIncrementZoom = {}, | ||
| captureButtonUiState = CaptureButtonUiState.Enabled.Idle( | ||
| CaptureMode.STANDARD, | ||
| isEnabled = false | ||
| ) | ||
| ) | ||
| } | ||
| composeTestRule.onRoot().tryPerformAccessibilityChecks() | ||
| composeTestRule.onNodeWithTag("CaptureButtonDisabled").assertExists() | ||
| composeTestRule.onNodeWithTag( | ||
| "CaptureButtonDisabled" | ||
| ).assert(androidx.compose.ui.test.isNotEnabled()) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should define the versions for these under
[versions]and reference them here instead