diff --git a/apps/mobile/src/Stack.tsx b/apps/mobile/src/Stack.tsx index ea46df3235f5835b3c9a9f5e5b9cbc9870e00f11..4dfa3f3a985698d9293a84c441834501b401a47b 100644 --- a/apps/mobile/src/Stack.tsx +++ b/apps/mobile/src/Stack.tsx @@ -51,6 +51,7 @@ import { useConnectOnboardingNavigation } from "./features/cloud/connectOnboardi import { AttachmentFileScreen } from "./features/files/AttachmentFileScreen"; import { ThreadFilesTreeScreen, ThreadFileScreen } from "./features/files/ThreadFilesRouteScreen"; import { AdaptiveWorkspaceLayout } from "./features/layout/AdaptiveWorkspaceLayout"; +import { selectedPullRequestKey } from "./features/pull-requests/pullRequestSelection"; import { HardwareKeyboardCommandOverlay, HardwareKeyboardCommandProvider, @@ -658,6 +659,7 @@ function RootStackLayout(props: { {props.children} diff --git a/apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx b/apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx index b750fd5f94a83869cb27d5c661ee895f03e2d198..b2981f63e2551bd3a694f460082e451dc2ee7f55 100644 --- a/apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx +++ b/apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx @@ -220,6 +220,7 @@ export function AdaptiveWorkspaceLayout(props: { readonly children: ReactNode; readonly pathname: string; readonly workspaceRouteKey: string | undefined; + readonly selectedPullRequestKey?: string | null; }) { const preferencesResult = useAtomValue(mobilePreferencesAtom); if (!AsyncResult.isSuccess(preferencesResult)) { @@ -245,6 +246,7 @@ function AdaptiveWorkspaceLayoutContent( readonly children: ReactNode; readonly pathname: string; readonly workspaceRouteKey: string | undefined; + readonly selectedPullRequestKey?: string | null; } & { readonly projectGroupingMode: SidebarProjectGroupingMode; readonly savedThreadListFilters?: ThreadListFilters | undefined; @@ -739,7 +741,10 @@ function AdaptiveWorkspaceLayoutContent( /> )} > - + void; readonly selected: boolean; + readonly sidebar: boolean; readonly busy: boolean; readonly getActions: (item: EnvironmentPullRequestListEntry) => ControlPillMenuProps["actions"]; readonly onAction: (item: EnvironmentPullRequestListEntry, action: string) => void; }) { const { item } = props; + const foreground = props.selected + ? selectedThreadRowColors.foregroundClassName + : props.sidebar + ? "text-drawer-foreground" + : "text-foreground"; + const mutedForeground = props.selected + ? selectedThreadRowColors.mutedForegroundClassName + : props.sidebar + ? "text-drawer-foreground-muted" + : "text-foreground-muted"; + const tertiaryForeground = props.selected + ? selectedThreadRowColors.mutedForegroundClassName + : props.sidebar + ? "text-drawer-foreground-muted" + : "text-foreground-tertiary"; const status = pullRequestStatus(item); const review = pullRequestListReviewStatus(item); const checkTint = @@ -34,123 +55,144 @@ export const PullRequestListRow = memo( ? "accent-danger-foreground" : "accent-warning-foreground"; return ( - props.onAction(item, nativeEvent.event)} - > - label.name).join(", ")}` : ""}, ${item.additions} additions, ${item.deletions} deletions`} - accessibilityHint="Double tap to open. Touch and hold for quick actions." - onPress={() => props.onSelect(item)} - disabled={props.busy} - className={cn( - "flex-row gap-3 px-4 active:bg-row-hover", - props.selected && "bg-thread-selected", - props.busy && "opacity-50", - )} + + props.onAction(item, nativeEvent.event)} > - - - - - - - - {item.repository} #{item.number} - - {item.stack ? ( - - - - {item.stack.position}/{item.stack.size} - - - ) : null} - - {relativeTime(item.updatedAt)} - + label.name).join(", ")}` : ""}, ${item.additions} additions, ${item.deletions} deletions`} + accessibilityHint="Double tap to open. Touch and hold for quick actions." + onPress={() => props.onSelect(item)} + disabled={props.busy} + style={props.sidebar ? THREAD_LIST_V2_SIDEBAR_ROW_STYLE : undefined} + className={cn( + "flex-row gap-3 overflow-hidden", + !props.sidebar && "px-4 active:bg-row-hover", + props.sidebar && "bg-drawer", + props.sidebar && !props.selected && "active:bg-thread-hover", + props.selected && "bg-thread-selected", + props.busy && "opacity-50", + )} + > + + - - {item.title} - - - - {item.labels.slice(0, 2).map((label) => { - const color = pullRequestLabelColor(label.color); - return ( - - {color ? ( - - ) : null} - + + + {item.repository} #{item.number} + + {item.stack ? ( + + + + {item.stack.position}/{item.stack.size} + + + ) : null} + + {relativeTime(item.updatedAt)} + + + + {item.title} + + + + {item.labels.slice(0, 2).map((label) => { + const color = pullRequestLabelColor(label.color); + return ( + - {label.name} - - - ); - })} - {item.labels.length > 2 ? ( - - +{item.labels.length - 2} - + {color ? ( + + ) : null} + + {label.name} + + + ); + })} + {item.labels.length > 2 ? ( + + +{item.labels.length - 2} + + ) : null} + + {item.checksState ? ( + + + ) : null} - - {item.checksState ? ( - - + + + ) : null} + {item.additions || item.deletions ? ( + - - ) : null} - {review ? ( - - - - ) : null} - {item.additions || item.deletions ? ( - - ) : null} + ) : null} + - - - + + + ); }, (previous, next) => pullRequestListRowsAreEqual(previous.item, next.item) && previous.selected === next.selected && + previous.sidebar === next.sidebar && previous.busy === next.busy && previous.onSelect === next.onSelect && previous.onAction === next.onAction && diff --git a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx index 5120db3d8508a8530e062848f10263363c72c2b0..c90cdc98e50155a8b865314f66a451f5010a5f3a 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx @@ -53,7 +53,12 @@ function PullRequestScrollView(props: ScrollViewProps) { ); } -export function PullRequestsScreen() { +export function PullRequestsScreen( + props: { + readonly sidebar?: boolean; + readonly selectedPullRequestKey?: string | null; + } = {}, +) { const navigation = useNavigation(); const primaryColumn = use(NativePrimaryColumnContext); const workspace = useWorkspaceTabs()!; @@ -135,7 +140,8 @@ export function PullRequestsScreen() { connectedTargets.map((target) => target.environmentId), result.updateEntry, ); - const selectedKey = primaryColumn?.selectedPullRequestKey; + const sidebar = props.sidebar === true || primaryColumn !== null; + const selectedKey = props.selectedPullRequestKey ?? primaryColumn?.selectedPullRequestKey; const entries = useMemo( () => filterPullRequestList( @@ -179,9 +185,10 @@ export function PullRequestsScreen() { onAction={select} busy={busyKey === pullRequestKey(item)} selected={selectedKey === pullRequestKey(item)} + sidebar={sidebar} /> ), - [selectPullRequest, selectedKey, actions, select, busyKey], + [selectPullRequest, selectedKey, actions, select, busyKey, sidebar], ); const previousSearchMode = useRef(tab.searchMode); useEffect(() => { @@ -199,11 +206,11 @@ export function PullRequestsScreen() { : -1; return ( - + { + it("normalizes repository and host while retaining the environment scope", () => { + expect(selection([home, pullRequest])).toBe("environment-one:github.com:org/repo:42"); + expect( + selection([ + home, + { ...pullRequest, params: { ...params, environmentId: "environment-two" } }, + ]), + ).toBe("environment-two:github.com:org/repo:42"); + }); + + it.each([ + "PullRequestFile", + "PullRequestFiles", + "PullRequestGuideStop", + "PullRequestDiscussion", + "PullRequestReviewers", + "PullRequestReview", + "PullRequestInlineComment", + ])("keeps the owning selection on %s", (name) => { + expect(selection([home, pullRequest, { key: name, name, params }])).toBe( + "environment-one:github.com:org/repo:42", + ); + }); + + it("uses the most recently opened PR, including a different hosting service", () => { + expect( + selection([ + home, + pullRequest, + { + key: "other-pr", + name: "PullRequest", + params: { ...params, host: "github.enterprise", repository: "Other/Repo", number: 7 }, + }, + ]), + ).toBe("environment-one:github.enterprise:other/repo:7"); + }); + + it.each(["Home", "Thread", "ThreadFiles"])("clears an older selection on %s", (name) => { + expect(selection([home, pullRequest, { key: "current", name }])).toBeNull(); + }); + + it("ignores inactive routes after the navigation index", () => { + expect(selection([home, pullRequest], 0)).toBeNull(); + }); + + it("retains the owning PR behind an unrelated overlay", () => { + expect(selection([home, pullRequest, { key: "settings", name: "SettingsSheet" }])).toBe( + "environment-one:github.com:org/repo:42", + ); + }); + + it("does not highlight an older PR when the current PR lacks its hosting identity", () => { + expect( + selection([ + home, + pullRequest, + { key: "unresolved", name: "PullRequest", params: { environmentId: "environment-one" } }, + ]), + ).toBeNull(); + }); +}); diff --git a/apps/mobile/src/features/pull-requests/pullRequestSelection.ts b/apps/mobile/src/features/pull-requests/pullRequestSelection.ts new file mode 100644 index 0000000000000000000000000000000000000000..fa31d1de91601c261ee504a67b46d172b5d25e48 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/pullRequestSelection.ts @@ -0,0 +1,32 @@ +import type { NavigationState } from "@react-navigation/native"; +import { pullRequestListKey } from "./pullRequestList"; + +/** Follow the displayed PR through its detail routes and sheets, not an older stack entry. */ +export function selectedPullRequestKey(state: Pick) { + for (let index = state.index; index >= 0; index--) { + const route = state.routes[index]; + if (!route) continue; + if (route.name === "Home" || route.name.startsWith("Thread")) return null; + if (!route.name.startsWith("PullRequest")) continue; + const params = route.params; + if ( + params && + "environmentId" in params && + "host" in params && + "repository" in params && + "number" in params && + typeof params.environmentId === "string" && + typeof params.host === "string" && + typeof params.repository === "string" && + typeof params.number === "number" + ) { + return `${params.environmentId}:${pullRequestListKey({ + host: params.host, + repository: params.repository, + number: params.number, + })}`; + } + if (route.name === "PullRequest") return null; + } + return null; +} diff --git a/apps/mobile/src/features/threads/thread-list-v2-row-appearance.android.ts b/apps/mobile/src/features/threads/thread-list-v2-row-appearance.android.ts index 8be6b240755dbe52597d62d0e4a293d3013749a5..c63d327764cb6e8bc68539336cb58f894594d9c5 100644 --- a/apps/mobile/src/features/threads/thread-list-v2-row-appearance.android.ts +++ b/apps/mobile/src/features/threads/thread-list-v2-row-appearance.android.ts @@ -4,6 +4,17 @@ import type { MobileThemeVariables } from "../../lib/mobileTheme"; export const THREAD_LIST_V2_MONO_FONT = "monospace"; export const THREAD_LIST_V2_ROW_CONTENT_CLASS_NAME = "px-3 py-2.5"; export const THREAD_LIST_V2_ROW_DIVIDERS = false; +export const THREAD_LIST_V2_SIDEBAR_ROW_STYLE = { + borderRadius: 20, + paddingHorizontal: 12, + paddingVertical: 10, +} satisfies ViewStyle; +export const THREAD_LIST_V2_SIDEBAR_ROW_CONTAINER_STYLE = { + borderRadius: 20, + overflow: "hidden", + marginHorizontal: 8, + marginVertical: 2, +} satisfies ViewStyle; export const selectedThreadRowColors = { foregroundClassName: "text-thread-selected-foreground", @@ -25,14 +36,9 @@ export function getThreadListV2RowAppearance( const backgroundColor = theme[sidebarPane ? "--color-drawer" : "--color-screen"]; const style: ViewStyle = { backgroundColor: selected ? selectedBackgroundColor : backgroundColor, - borderRadius: 20, - }; - const swipeContainerStyle: ViewStyle = { - borderRadius: 20, - overflow: "hidden", - marginHorizontal: 8, - marginVertical: 2, + borderRadius: THREAD_LIST_V2_SIDEBAR_ROW_STYLE.borderRadius, }; + const swipeContainerStyle = THREAD_LIST_V2_SIDEBAR_ROW_CONTAINER_STYLE; return { className: undefined, @@ -52,7 +58,7 @@ export function getThreadListV2RowAppearance( ? "accent-drawer-foreground-muted" : "accent-foreground-tertiary", style, - cardStyle: sidebarPane ? { ...style, paddingHorizontal: 12, paddingVertical: 10 } : style, + cardStyle: sidebarPane ? { ...style, ...THREAD_LIST_V2_SIDEBAR_ROW_STYLE } : style, swipeContainerStyle, swipeBackgroundColor: backgroundColor, providerIconSurfaceColor: selected ? selectedBackgroundColor : backgroundColor, diff --git a/apps/mobile/src/features/threads/thread-list-v2-row-appearance.ts b/apps/mobile/src/features/threads/thread-list-v2-row-appearance.ts index b73f87b86a2ad45c5922430595a3b537e3cb2bac..1b63ff57d65f4d01be76cdfc207fd603ec5672bc 100644 --- a/apps/mobile/src/features/threads/thread-list-v2-row-appearance.ts +++ b/apps/mobile/src/features/threads/thread-list-v2-row-appearance.ts @@ -4,6 +4,15 @@ import type { MobileThemeVariables } from "../../lib/mobileTheme"; export const THREAD_LIST_V2_MONO_FONT = "Menlo"; export const THREAD_LIST_V2_ROW_CONTENT_CLASS_NAME = "px-5 py-2.5"; export const THREAD_LIST_V2_ROW_DIVIDERS = true; +export const THREAD_LIST_V2_SIDEBAR_ROW_STYLE = { + borderRadius: 12, + paddingHorizontal: 12, + paddingVertical: 10, +} satisfies ViewStyle; +export const THREAD_LIST_V2_SIDEBAR_ROW_CONTAINER_STYLE = { + borderRadius: 12, + overflow: "hidden", +} satisfies ViewStyle; export const selectedThreadRowColors = { foregroundClassName: "text-thread-selected-foreground", @@ -25,11 +34,11 @@ export function getThreadListV2RowAppearance( const style: ViewStyle | undefined = sidebarPane ? { backgroundColor: selected ? selectedBackgroundColor : theme["--color-drawer"], - borderRadius: 12, + borderRadius: THREAD_LIST_V2_SIDEBAR_ROW_STYLE.borderRadius, } : undefined; const swipeContainerStyle: ViewStyle | undefined = sidebarPane - ? { borderRadius: 12, overflow: "hidden" } + ? THREAD_LIST_V2_SIDEBAR_ROW_CONTAINER_STYLE : undefined; return { @@ -50,7 +59,7 @@ export function getThreadListV2RowAppearance( ? "accent-drawer-foreground-muted" : "accent-foreground-tertiary", style, - cardStyle: sidebarPane ? { ...style, paddingHorizontal: 12, paddingVertical: 10 } : undefined, + cardStyle: sidebarPane ? { ...style, ...THREAD_LIST_V2_SIDEBAR_ROW_STYLE } : undefined, swipeContainerStyle, swipeBackgroundColor: theme[sidebarPane ? "--color-drawer" : "--color-screen"], // Provider badges blend into the surface beneath them. diff --git a/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.android.tsx b/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.android.tsx index 8d65314da12d5fa9ad52e83cd4b4eba71260376c..711f8cbae5aa91cad07f069d428d27934dc39ae6 100644 --- a/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.android.tsx +++ b/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.android.tsx @@ -8,6 +8,7 @@ import { WorkspaceTabsContext, useWorkspaceTabs } from "./WorkspaceTabsContext"; export function AndroidWorkspaceSidebar(props: { readonly children: ReactNode; readonly onRequestVisibility: () => void; + readonly selectedPullRequestKey: string | null; }) { const workspace = useWorkspaceTabs()!; const context = useMemo(() => ({ ...workspace, bottomClearance: 16 }), [workspace]); @@ -18,7 +19,7 @@ export function AndroidWorkspaceSidebar(props: { - + diff --git a/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.tsx b/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.tsx index b31fadadbce22d94a235e423fa40df7a675db603..6dbfa1cfa949c5f42aa4b29494d52d464553c281 100644 --- a/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.tsx +++ b/apps/mobile/src/features/workspace/AndroidWorkspaceSidebar.tsx @@ -3,6 +3,7 @@ import type { ReactNode } from "react"; export function AndroidWorkspaceSidebar(props: { readonly children: ReactNode; readonly onRequestVisibility: () => void; + readonly selectedPullRequestKey: string | null; }) { return props.children; }