Add PreviewFocusWithFocusGroup Preview to test focus state#778
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new preview composable demonstrating focus-group behavior and focus requesters, integrates kotlinx-coroutines-test as a compileOnly dependency, opts into ExperimentalTestApi in test helpers, and updates test-rule setup to use StandardTestDispatcher and advance the main clock before capture. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Preview as PreviewComposable
participant Launched as LaunchedEffect
participant OuterFR as OuterFocusRequester
participant FocusGroup as FocusGroup(Row)
participant InnerFR as InnerFocusRequester
participant TF1 as BasicTextField#1
participant TF2 as BasicTextField#2
Note over Preview,TF2: Composition of PreviewFocusGroupLaunchedEffectMinimal
Preview->>Launched: start LaunchedEffect
Launched->>OuterFR: requestFocus()
OuterFR->>FocusGroup: deliver focus to group
FocusGroup->>InnerFR: transfer focus to inner requester
InnerFR->>TF1: TF1 gains focus
TF1-->>Preview: onFocusChanged(true)
Preview-->>TF1: update text/background state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt (1)
462-491: Well-implemented test for LaunchedEffect + FocusRequester pattern.The implementation correctly demonstrates:
- Proper use of
rememberforFocusRequesterand state preservationLaunchedEffectwithUnitkey to request focus on initial composition- Correct modifier ordering (
focusRequesterbeforeonFocusChanged)- Focus state tracking that triggers recomposition and visual feedback
The test effectively verifies that
LaunchedEffectproperly triggers focus state changes in preview tests.Optional: Improve text visibility
For better visual clarity in the test, consider specifying an explicit text color that ensures good contrast on both focused (blue) and unfocused (gray) backgrounds:
BasicTextField( value = "Focus test", onValueChange = {}, + textStyle = androidx.compose.ui.text.TextStyle(color = Color.White), modifier = Modifier .focusRequester(focusRequester) .onFocusChanged { isFocused = it.isFocused } .background(if (isFocused) Color.Blue else Color.Gray) .padding(8.dp) )This ensures the text remains clearly visible on both background colors during testing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: compare-screenshot-test
- GitHub Check: store-screenshot-test
- GitHub Check: report
- GitHub Check: ollama-test
- GitHub Check: build
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt (1)
25-25: LGTM! Imports are appropriate for the new functionality.The new imports for
BasicTextField,FocusRequester,focusRequester, andonFocusChangedare all correctly used in the newPreviewLaunchedEffectFocusfunction.Also applies to: 32-34
|
Snapshot diff report
|
2a60e34 to
5acf700
Compare
- Remove ComposeRuleFactoryParams class (unnecessary indirection) - Remove effectDispatcherFactory from JUnit4TestLifecycleOptions - Change composeRuleFactory signature from (ComposeRuleFactoryParams) -> to () -> - Create StandardTestDispatcher internally in default composeRuleFactory - Remove effectDispatcher parameter from composeTestRule() and RoborazziComposeTestRuleOption - Replace scheduler.advanceUntilIdle() with mainClock.advanceTimeByFrame()
…t-dispatcher-support
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt (2)
6-7: Remove unused imports for cleaner code.The imports
focusGroup,focusable,Row, andfocusRestorerare not used in the implementation. Consider removing them to keep the imports clean.🧹 Proposed cleanup
-import androidx.compose.foundation.focusGroup -import androidx.compose.foundation.focusable -import androidx.compose.foundation.layout.Row import androidx.compose.foundation.text.BasicTextField import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester -import androidx.compose.ui.focus.focusRestorer import androidx.compose.ui.focus.onFocusChangedAlso applies to: 12-12, 37-37
469-469: Consider renaming to match actual implementation.The function name
PreviewFocusWithFocusGroupMixedMinimalTinymentions "FocusGroup" but doesn't use thefocusGroupmodifier. Consider a name likePreviewFocusWithLaunchedEffectorPreviewBasicTextFieldFocusto better reflect what's being tested.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: report
- GitHub Check: store-screenshot-test
- GitHub Check: compare-screenshot-test
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (2)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt (2)
470-486: Implementation looks correct for testing focus behavior.The logic correctly demonstrates:
- Using
FocusRequesterto programmatically request focus- Tracking focus state via
onFocusChanged- Visual feedback showing "OK" with blue background when focused, "NG" with gray when not
The
LaunchedEffect(Unit)will trigger focus request after composition, causing a state update and recomposition—similar to the pattern inPreviewOnSizeChangedabove. This should work correctly if Roborazzi waits for recomposition before capture.
466-466: Remove this review comment – the referenced code does not exist.The code snippet shown in this review comment (
@OptIn(androidx.compose.ui.ExperimentalComposeUiApi::class)) is not present at line 466 or anywhere else in the file. Line 466 contains a comment about LaunchedEffect focus timing. The functionPreviewFocusLaunchedEffectMinimalexists but does not use this annotation. This review appears to be based on outdated or incorrect information.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt:
- Line 467: Remove the unnecessary
@OptIn(androidx.compose.ui.ExperimentalComposeUiApi::class) annotation in
Previews.kt since focusGroup is now stable; delete the annotation line above the
affected declaration (the @OptIn on the preview/composable declaration) and also
remove any now-unused import of androidx.compose.ui.ExperimentalComposeUiApi if
it becomes unused.
🧹 Nitpick comments (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt (1)
471-499: Consider clarifying the focus request logic.The
innerFocusRequester is created and attached to the firstBasicTextFieldbut is never used to request focus—onlyouter.requestFocus()is called on theRowwithfocusGroup().If this is a minimal reproduction case demonstrating specific focus behavior (as the comments suggest), the current implementation is fine. However, if the intent is to directly focus the first
BasicTextField, consider callinginner.requestFocus()instead ofouter.requestFocus().💡 Alternative: Request focus directly on the child element
If the goal is to focus the first
BasicTextFieldexplicitly:LaunchedEffect(Unit) { - outer.requestFocus() + inner.requestFocus() }Note: Only apply this change if direct child focus is the desired behavior, not if you're reproducing the current focusGroup behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: ollama-test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: report
- GitHub Check: store-screenshot-test
- GitHub Check: compare-screenshot-test
| // Minimal repro: focusGroup + two children + LaunchedEffect | ||
| // - Android Studio Preview: OK | ||
| // - Robolectric without TestDispatcher: NG | ||
| @OptIn(androidx.compose.ui.ExperimentalComposeUiApi::class) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
androidx.compose.ui focusGroup API stability status ExperimentalComposeUiApi
💡 Result:
- Modifier.focusGroup is stable — it was promoted to stable APIs in androidx.compose.ui 1.6.0. [1]. (developer.android.com)
- The focusGroup modifier is documented as available (Modifier.focusGroup()) in the Compose docs/foundation API. [2]. (composables.com)
- The ExperimentalComposeUiApi annotation still exists for other focus-related helpers (e.g., older/deprecated focusRestorer overloads) but focusGroup itself is not experimental. [3]. (developer.android.com)
Sources: [1] AndroidX Compose UI release notes; [2] focusGroup API docs; [3] androidx.compose.ui.focus package reference.
Citations:
- 1: https://developer.android.com/jetpack/androidx/releases/compose-ui?utm_source=openai
- 2: https://composables.com/docs/androidx.compose.foundation/foundation/modifiers/focusGroup?utm_source=openai
- 3: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/package-summary?utm_source=openai
🏁 Script executed:
# Check the file location and context to confirm it's test/sample code
head -20 sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt
# Look for additional context around line 467
sed -n '460,475p' sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.ktRepository: takahirom/roborazzi
Length of output: 1438
Remove unnecessary @OptIn annotation — focusGroup is stable.
The focusGroup modifier was promoted to stable in androidx.compose.ui 1.6.0 and is no longer experimental. The @OptIn(androidx.compose.ui.ExperimentalComposeUiApi::class) annotation can be removed.
🤖 Prompt for AI Agents
In
@sample-generate-preview-tests/src/main/java/com/github/takahirom/preview/tests/Previews.kt
at line 467, Remove the unnecessary
@OptIn(androidx.compose.ui.ExperimentalComposeUiApi::class) annotation in
Previews.kt since focusGroup is now stable; delete the annotation line above the
affected declaration (the @OptIn on the preview/composable declaration) and also
remove any now-unused import of androidx.compose.ui.ExperimentalComposeUiApi if
it becomes unused.
…t-dispatcher-support
Add effectDispatcherFactory for LaunchedEffect support
What
Add a new Preview function
PreviewLaunchedEffectFocusthat tests LaunchedEffect with FocusRequester.Why
To verify that LaunchedEffect properly triggers focus state changes in preview tests.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.