From 63923fdd07bf1053cd0567a32ae5c1e9e200a58b Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 31 Aug 2026 12:10:40 +0200 Subject: [PATCH] Keep Android timeline still when tapping text (#4090) * fix(android): keep timeline still when tapping text Selectable Android text takes touch focus. React Native then scrolls focused children using untransformed coordinates, which moves inverted timelines even though the user did not scroll.\n\nSkip that auto-scroll only for touch-focused, selectable, non-editable text in inverted vertical scroll views, preserving normal focus navigation and editable inputs. * test(e2e): isolate PR pane fixtures from bots --- packages/app/e2e/browser/pr-pane.spec.ts | 25 +++++------ packages/app/e2e/support/helpers/pr-pane.ts | 41 +++++++++++-------- .../git/pull-request-panel/checks-section.tsx | 2 +- patches/react-native+0.81.5.patch | 37 +++++++++++++++++ scripts/postinstall-patches.mjs | 4 ++ 5 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 patches/react-native+0.81.5.patch diff --git a/packages/app/e2e/browser/pr-pane.spec.ts b/packages/app/e2e/browser/pr-pane.spec.ts index 1dd94b6737..acb5fe39ef 100644 --- a/packages/app/e2e/browser/pr-pane.spec.ts +++ b/packages/app/e2e/browser/pr-pane.spec.ts @@ -3,8 +3,8 @@ import { openPrPane, expectPrPaneTitle, expectPrPaneState, - expectPrPaneCheckSummary, - expectPrPaneActivityCount, + expectPrPaneChecks, + expectPrPaneActivity, } from "../support/helpers/pr-pane"; import { gotoWorkspace } from "../support/helpers/launcher"; import { @@ -50,7 +50,6 @@ test.describe("PR pane", () => { ], }, { title: "PR with reviews", state: "open", commentCount: 3 }, - { title: "PR with no checks", state: "open" }, ], }); @@ -102,25 +101,21 @@ test.describe("PR pane", () => { await expectPrPaneTitle(page, "Work in progress"); }); - test("renders check pills with correct success/failure/pending counts", async ({ page }) => { + test("renders seeded checks in their success, failure, and pending groups", async ({ page }) => { await gotoWorkspace(page, workspaceByTitle.get("PR with mixed checks")!); await openPrPane(page); - await expectPrPaneCheckSummary(page, { success: 2, failure: 1, pending: 1 }); + await expectPrPaneChecks(page, { + success: ["build-1", "build-2"], + failure: ["deploy"], + pending: ["security"], + }); }); - test("renders activity rows with correct count", async ({ page }) => { + test("renders every seeded activity comment", async ({ page }) => { await gotoWorkspace(page, workspaceByTitle.get("PR with reviews")!); await openPrPane(page); - await expectPrPaneActivityCount(page, 3); - }); - - test("renders gracefully with zero checks", async ({ page }) => { - await gotoWorkspace(page, workspaceByTitle.get("PR with no checks")!); - await openPrPane(page); - - await expectPrPaneCheckSummary(page, { success: 0, failure: 0, pending: 0 }); - await expectPrPaneTitle(page, "PR with no checks"); + await expectPrPaneActivity(page, ["Test comment 1", "Test comment 2", "Test comment 3"]); }); }); diff --git a/packages/app/e2e/support/helpers/pr-pane.ts b/packages/app/e2e/support/helpers/pr-pane.ts index 42aa200fdd..56f0ef444c 100644 --- a/packages/app/e2e/support/helpers/pr-pane.ts +++ b/packages/app/e2e/support/helpers/pr-pane.ts @@ -1,5 +1,4 @@ import { expect, type Page } from "@playwright/test"; -import type { CheckPresentationCounts } from "@/git/check-presentation"; import { getStateLabel } from "@/git/pull-request-panel/data"; import { openPullRequestPanel } from "./workspace-tabs"; @@ -20,23 +19,31 @@ export async function expectPrPaneState( }); } -async function assertCheckPill(page: Page, testId: string, count: number): Promise { - const locator = page.getByTestId(testId); - await expect(locator).toHaveCount(count > 0 ? 1 : 0, { timeout: 15_000 }); - if (count > 0) { - await expect(locator).toContainText(String(count)); +export async function expectPrPaneChecks( + page: Page, + checks: { success: string[]; failure: string[]; pending: string[] }, +): Promise { + const section = page.getByTestId("pr-pane-checks"); + const groups = [ + { status: "failure", label: /failing checks?$/, names: checks.failure }, + { status: "pending", label: /in progress checks?$/, names: checks.pending }, + { status: "success", label: /successful checks?$/, names: checks.success }, + ]; + + for (const group of groups) { + const groupSection = section.getByTestId(`pr-pane-check-group-${group.status}`); + await expect(groupSection.getByRole("button", { name: group.label })).toBeVisible({ + timeout: 15_000, + }); + for (const name of group.names) { + await expect(groupSection.getByText(name, { exact: true })).toHaveCount(1); + } } } -export async function expectPrPaneCheckSummary( - page: Page, - counts: Pick, -): Promise { - await assertCheckPill(page, "pr-pane-check-success", counts.success); - await assertCheckPill(page, "pr-pane-check-failure", counts.failure); - await assertCheckPill(page, "pr-pane-check-pending", counts.pending); -} - -export async function expectPrPaneActivityCount(page: Page, count: number): Promise { - await expect(page.getByTestId("pr-pane-activity-row")).toHaveCount(count, { timeout: 15_000 }); +export async function expectPrPaneActivity(page: Page, bodies: string[]): Promise { + const activity = page.getByTestId("pr-pane-activity-row"); + for (const body of bodies) { + await expect(activity.getByText(body, { exact: true })).toBeVisible({ timeout: 15_000 }); + } } diff --git a/packages/app/src/git/pull-request-panel/checks-section.tsx b/packages/app/src/git/pull-request-panel/checks-section.tsx index 39ad9a0b96..9e4f2dd942 100644 --- a/packages/app/src/git/pull-request-panel/checks-section.tsx +++ b/packages/app/src/git/pull-request-panel/checks-section.tsx @@ -148,7 +148,7 @@ function CheckGroup({ }) { const handlePress = useCallback(() => onToggle(group.status), [group.status, onToggle]); return ( - + {group.label} {collapsed ? ( diff --git a/patches/react-native+0.81.5.patch b/patches/react-native+0.81.5.patch new file mode 100644 index 0000000000..41c0d868f0 --- /dev/null +++ b/patches/react-native+0.81.5.patch @@ -0,0 +1,37 @@ +diff --git a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +index f708543..26df5e0 100644 +--- a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java ++++ b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +@@ -29,6 +29,7 @@ import android.view.ViewGroup; + import android.view.accessibility.AccessibilityNodeInfo; + import android.widget.OverScroller; + import android.widget.ScrollView; ++import android.widget.TextView; + import androidx.annotation.NonNull; + import androidx.annotation.Nullable; + import androidx.core.view.ViewCompat; +@@ -425,12 +426,23 @@ public class ReactScrollView extends ScrollView + */ + @Override + public void requestChildFocus(View child, View focused) { +- if (focused != null) { ++ if (focused != null && shouldScrollToFocusedChild(focused)) { + scrollToChild(focused); + } + super.requestChildFocus(child, focused); + } + ++ private boolean shouldScrollToFocusedChild(View focused) { ++ if (getScaleY() >= 0 || !focused.isInTouchMode() || !(focused instanceof TextView)) { ++ return true; ++ } ++ ++ // Selectable text takes focus when tapped. In inverted lists, scrolling to that focus uses ++ // untransformed coordinates and unexpectedly moves the timeline. ++ TextView textView = (TextView) focused; ++ return !textView.isTextSelectable() || textView.onCheckIsTextEditor(); ++ } ++ + private int getScrollDelta(View descendent) { + descendent.getDrawingRect(mTempRect); + offsetDescendantRectToMyCoords(descendent, mTempRect); diff --git a/scripts/postinstall-patches.mjs b/scripts/postinstall-patches.mjs index 2a762c7a5f..53b8395480 100644 --- a/scripts/postinstall-patches.mjs +++ b/scripts/postinstall-patches.mjs @@ -12,6 +12,10 @@ const patchedPackages = [ nodeModulesPath: "node_modules/react-native-markdown-display", patchPrefix: "react-native-markdown-display+", }, + { + nodeModulesPath: "node_modules/react-native", + patchPrefix: "react-native+", + }, { nodeModulesPath: "node_modules/react-native-draggable-flatlist", patchPrefix: "react-native-draggable-flatlist+",