ReviewOS
Reload

also looking at this

craft-native/craft

fix(native): retain keyed controls and correct map and tab chrome

#368
Merged chris wants to merge codex/native-repeated-selector-identity into main
32 files +449 -100

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

.github/workflows/mobile-e2e.ymlmodified+2-2
@@ -76,7 +76,7 @@ on:
7676 android_suite:
7777 description: Android native fixture coverage for the android-navigation scope
7878 type: choice
79 options: [focused, full, system-bars, maps, accessibility]
79 options: [focused, full, system-bars, maps, accessibility, identity]
8080 default: focused
8181 selectors:
8282 description: Optional XCTest selector for focused native iOS rendering or navigation
@@ -618,7 +618,7 @@ jobs:
618618 api-level: ${{ fromJSON((inputs.scope != 'android-navigation' || inputs.android_suite == 'full') && '[34,35]' || inputs.android_suite == 'system-bars' && '[35]' || '[34]') }}
619619 env:
620620 CRAFT_ANDROID_NATIVE_SUITE: ${{ inputs.scope != 'android-navigation' && 'full' || inputs.android_suite || 'focused' }}
621 CRAFT_ANDROID_TEST_CLASS: ${{ inputs.scope == 'android-navigation' && inputs.android_suite == 'accessibility' && 'dev.craft.navigationtest.NativeNavigationTest#nativeTestIdsRemainQueryableWithoutReplacingVisibleLabels' || inputs.scope == 'android-navigation' && inputs.android_suite == 'maps' && 'dev.craft.navigationtest.NativeMapTest#keyFreeBasemapsKeepTheRecordedRouteAndDirectionSignsVisible,dev.craft.navigationtest.NativeMapTest#nativeMapMutationsUpdateThePinAndCameraWithoutReloadingTheStyle,dev.craft.navigationtest.NativeMapTest#showingTheLocationDotDoesNotRequestPermissionDuringRendering,dev.craft.navigationtest.NativeMapTest#controlledRegionEchoPreservesTheCameraAfterANativeDrag,dev.craft.navigationtest.NativeMapTest#backgroundAndResumeKeepTheNativeMapAndCamera' || inputs.scope == 'android-navigation' && inputs.android_suite == 'system-bars' && 'dev.craft.navigationtest.NativeNavigationTest#nativeScreenBackgroundOptionAppliesAndResets' || inputs.scope == 'android-navigation' && inputs.android_suite != 'full' && 'dev.craft.navigationtest.NativeNavigationTest#outgoingTabTapWinsOverSettlingSwipe,dev.craft.navigationtest.NativeNavigationTest#thirdTabTapWinsOverSettlingSwipe,dev.craft.navigationtest.NativeNavigationTest#cancelledSwipeTargetIsVisibleWhenSelectedLater,dev.craft.navigationtest.NativeNavigationTest#completedReverseSwipeKeepsRetainedTabVisible,dev.craft.navigationtest.NativeNavigationTest#nativePullToRefreshSurvivesViewReattachment,dev.craft.navigationtest.NativeNavigationTest#pushedScreenTabAccessoryOpensNativeRouteAndClearsOnBack,dev.craft.navigationtest.NativeFlatListTest#refreshAppearanceUsesTheScreenContrastFallback,dev.craft.navigationtest.NativeNavigationTest#nativeDatePickerKeepsFollowingControlsMounted' || '' }}
621 CRAFT_ANDROID_TEST_CLASS: ${{ inputs.scope == 'android-navigation' && inputs.android_suite == 'accessibility' && 'dev.craft.navigationtest.NativeNavigationTest#nativeTestIdsRemainQueryableWithoutReplacingVisibleLabels' || inputs.scope == 'android-navigation' && inputs.android_suite == 'maps' && 'dev.craft.navigationtest.NativeMapTest#keyFreeBasemapsKeepTheRecordedRouteAndDirectionSignsVisible,dev.craft.navigationtest.NativeMapTest#nativeMapMutationsUpdateThePinAndCameraWithoutReloadingTheStyle,dev.craft.navigationtest.NativeMapTest#showingTheLocationDotDoesNotRequestPermissionDuringRendering,dev.craft.navigationtest.NativeMapTest#controlledRegionEchoPreservesTheCameraAfterANativeDrag,dev.craft.navigationtest.NativeMapTest#backgroundAndResumeKeepTheNativeMapAndCamera' || inputs.scope == 'android-navigation' && inputs.android_suite == 'identity' && 'dev.craft.navigationtest.NativeNavigationTest#repeatedTestIdsKeepDistinctProtocolViewsAcrossReorders' || inputs.scope == 'android-navigation' && inputs.android_suite == 'system-bars' && 'dev.craft.navigationtest.NativeNavigationTest#nativeScreenBackgroundOptionAppliesAndResets' || inputs.scope == 'android-navigation' && inputs.android_suite != 'full' && 'dev.craft.navigationtest.NativeNavigationTest#outgoingTabTapWinsOverSettlingSwipe,dev.craft.navigationtest.NativeNavigationTest#thirdTabTapWinsOverSettlingSwipe,dev.craft.navigationtest.NativeNavigationTest#cancelledSwipeTargetIsVisibleWhenSelectedLater,dev.craft.navigationtest.NativeNavigationTest#completedReverseSwipeKeepsRetainedTabVisible,dev.craft.navigationtest.NativeNavigationTest#nativePullToRefreshSurvivesViewReattachment,dev.craft.navigationtest.NativeNavigationTest#pushedScreenTabAccessoryOpensNativeRouteAndClearsOnBack,dev.craft.navigationtest.NativeFlatListTest#refreshAppearanceUsesTheScreenContrastFallback,dev.craft.navigationtest.NativeNavigationTest#nativeDatePickerKeepsFollowingControlsMounted' || '' }}
622622 if: ${{ inputs.scope != 'ios-navigation' && inputs.scope != 'ios-render' }}
623623 runs-on: ubuntu-latest
624624 timeout-minutes: 35
packages/android/fixtures/native-navigation/NativeNavigationTest.ktmodified+47-0
@@ -773,6 +773,53 @@ class NativeNavigationTest {
773773 }
774774 }
775775
776 @Test
777 fun repeatedTestIdsKeepDistinctProtocolViewsAcrossReorders() {
778 val activity = instrumentation.startActivitySync(
779 Intent(instrumentation.targetContext, MainActivity::class.java)
780 .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK)
781 ) as MainActivity
782 fun rows(): List<View> {
783 val list = awaitView(activity, "identity-list")
784 val matches = mutableListOf<View>()
785 instrumentation.runOnMainSync {
786 findAll(list, "identity-row", matches)
787 }
788 assertEquals("Repeated test selectors must identify two separate rows", 2, matches.size)
789 return matches
790 }
791 try {
792 click(activity, "open-identity")
793 awaitView(activity, "identity-title")
794 val original = rows()
795 val alpha = original[0]
796 val beta = original[1]
797 val alphaInput = find(alpha, "identity-input") as EditText
798 val betaInput = find(beta, "identity-input") as EditText
799 assertTrue("Repeated selectors aliased separate inputs", alphaInput !== betaInput)
800 instrumentation.runOnMainSync {
801 betaInput.requestFocus()
802 betaInput.setText("Beta private draft")
803 }
804 instrumentation.waitForIdleSync()
805 click(activity, "identity-reverse")
806 awaitView(activity, "identity-list") { list ->
807 find(list, "identity-row")?.let { find(it, "identity-label-beta") != null } == true
808 }
809 instrumentation.waitForIdleSync()
810 val reversed = rows()
811 assertSame("Reordered Beta row lost its protocol identity", beta, reversed[0])
812 assertSame("Reordered Alpha row lost its protocol identity", alpha, reversed[1])
813 assertSame("Reordering replaced the focused native input", betaInput, find(reversed[0], "identity-input"))
814 assertEquals("Beta private draft", betaInput.text.toString())
815 assertEquals("The second row received another row's draft", "", alphaInput.text.toString())
816 assertTrue("Reordering dropped native focus", betaInput.isFocused)
817 } finally {
818 instrumentation.runOnMainSync { activity.finishAndRemoveTask() }
819 instrumentation.waitForIdleSync()
820 }
821 }
822
776823 @Test
777824 fun outgoingTabTapWinsOverSettlingSwipe() {
778825 assertTabTapInterruptsSettlingSwipe(0, "navigation-home-state", "Home state: Ada/1")
packages/android/scripts/test-native-navigation.tsmodified+1-0
@@ -34,6 +34,7 @@ try {
3434 mapProvider: 'maplibre',
3535 nativeScreens: {
3636 '/': 'home',
37 '/identity': 'identity',
3738 '/details': 'details',
3839 '/linked-details': 'details',
3940 '/summary': 'summary',
packages/ios/fixtures/native-navigation/Home.stxmodified+1-0
@@ -568,6 +568,7 @@ function openDetailsThenUpdateBackground() {
568568 <Button testID="keyed-list-reorder" @click="reorderNavigationRows()">Reorder rows</Button>
569569 <Text testID="navigation-home-state" :text="'Home state: ' + navigationName + '/' + navigationCount" />
570570 <Button testID="navigation-increment" @click="incrementNavigation()">Increment navigation state</Button>
571 <Button testID="open-identity" @click="globalThis.craft.navigation.open('/identity')">Repeated selector identity</Button>
571572 <Button testID="navigation-open-details" @click="openDetails()">Open navigation details</Button>
572573 <Button testID="navigation-check-lifecycle" @click="verifyNavigationLifecycle()">Check details lifecycle</Button>
573574 <Text testID="navigation-lifecycle" :text="navigationLifecycle" />
packages/ios/fixtures/native-navigation/Identity.stxadded+21-0
@@ -0,0 +1,21 @@
1<script>
2const rows = state([{ id: 'alpha', label: 'Alpha' }, { id: 'beta', label: 'Beta' }])
3const drafts = state({ alpha: '', beta: '' })
4function changeDraft(id, event) {
5 drafts.set({ ...drafts(), [id]: event.text })
6}
7function reverseRows() {
8 rows.set([...rows()].reverse())
9}
10</script>
11
12<SafeAreaView style={{"flex":1,"padding":16}}>
13 <Text testID="identity-title">Repeated selectors</Text>
14 <View testID="identity-list">
15 <View :for="row in rows" :key="row.id" testID="identity-row" style={{"height":72}}>
16 <Text :testID="'identity-label-' + row.id" :text="row.label" />
17 <TextInput testID="identity-input" :accessibilityLabel="row.label + ' draft'" :value="drafts()[row.id]" @input="changeDraft(row.id, $event)" />
18 </View>
19 </View>
20 <Button testID="identity-reverse" @click="reverseRows()">Reverse rows</Button>
21</SafeAreaView>
packages/ios/fixtures/native-navigation/native.config.jsonmodified+1-0
@@ -2,6 +2,7 @@
22 "initialScreen": "home",
33 "screens": {
44 "home": "Home.stx",
5 "identity": "Identity.stx",
56 "capabilities": "Capabilities.stx",
67 "details": "Details.stx",
78 "summary": "Summary.stx"
packages/ios/scripts/test-native-navigation.tsmodified+1-0
@@ -66,6 +66,7 @@ try {
6666 renderer: 'native',
6767 nativeScreens: {
6868 '/': 'home',
69 '/identity': 'identity',
6970 '/details': 'details',
7071 '/linked-details': 'details',
7172 '/summary': 'summary',