feat(mobile): bring GitLab and Bitbucket pull request review to GitHub parity (stack 25/30)

apps/mobile/src/components/pr-review/diff/pr-diff-file-list.test.tsx

@@ -4,10 +4,6 @@ import { type RefreshControlProps } from 'react-native';
 import TestRenderer, { act } from 'react-test-renderer';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
-import {
-  PR_DIFF_FLOATING_ACTIONS_FALLBACK_HEIGHT,
-  PR_DIFF_LIST_FOOTER_GAP,
-} from '@/lib/pr-review/diff/pr-diff-list-bottom-padding';
 import { ProviderPrScopeProvider } from '@/lib/pr-review/provider-pr-ref';
 
 import { PrReviewFileList } from './pr-diff-file-list';
@@ -318,16 +314,15 @@ describe('PrReviewFileList write affordances per provider', () => {
     });
   });
 
-  it('reserves the bar-sized gap under a provider diff list too', () => {
+  it('keeps the small footer gap under a provider diff list too', () => {
     const githubPadding = listBottomPadding(mountList());
     const gitlabPadding = listBottomPadding(
       mountListInScope({ platform: 'gitlab', projectPath: 'group/repo', mrIid: 12 })
     );
-    // The bar renders on every provider (s6), so every list reserves the
-    // bar's fallback height plus the footer gap — the last diff row is
-    // never hidden under the bar.
-    const expected = PR_DIFF_FLOATING_ACTIONS_FALLBACK_HEIGHT + PR_DIFF_LIST_FOOTER_GAP;
-    expect(githubPadding).toBe(expected);
-    expect(gitlabPadding).toBe(expected);
+    // The bar is an in-flow footer below the list (spot check e3), so no
+    // row can ever scroll under it; the list only keeps a 12-point gap
+    // between its last row and the footer's top edge, on every provider.
+    expect(githubPadding).toBe(12);
+    expect(gitlabPadding).toBe(12);
   });
 });

apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx

@@ -12,8 +12,9 @@
 //   * S7a adds diff-line selection: tapping a line runs the pure
 //     `selectLine` reducer; the result is mirrored into the
 //     `diff-selection-bridge` (so the comment composer can read it on
-//     mount) and a floating action bar (`PrDiffFloatingActions`)
-//     hosts the "Comment" and "Finish review" affordances.
+//     mount) and a footer action bar (`PrDiffFloatingActions`) rendered
+//     in-flow below the list hosts the "Comment" and "Finish review"
+//     affordances.
 //
 // Cold first paint: FlashList mounts only after the first page of files is
 // present. The first-load waiting state is a plain skeleton outside the list
@@ -41,7 +42,6 @@ import {
 import { PrDiffFileListLoading } from '@/components/pr-review/diff/pr-diff-file-list-loading';
 import { PrDiffFloatingActions } from '@/components/pr-review/diff/pr-diff-floating-actions';
 import { usePrDiffStateCopy } from '@/components/pr-review/diff/pr-diff-state-copy';
-import { prDiffListBottomPadding } from '@/lib/pr-review/diff/pr-diff-list-bottom-padding';
 import { useProviderPrScope } from '@/lib/pr-review/provider-pr-ref';
 import { useDiffRenderItem } from '@/components/pr-review/diff/pr-diff-file-list-render';
 import { useDiffSelection } from '@/components/pr-review/diff/use-diff-selection';
@@ -60,6 +60,9 @@ import { clearDiffSelection } from '@/lib/pr-review/diff-selection-bridge';
 import { CenteredState } from '@/components/centered-state';
 import { useIsTablet } from '@/lib/hooks/use-is-tablet';
 
+// Gap between the last diff row and the in-flow footer bar's top edge.
+const PR_DIFF_LIST_FOOTER_GAP = 12;
+
 type PrReviewFileListProps = {
   readonly owner: string;
   readonly repo: string;
@@ -127,30 +130,13 @@ export function PrReviewFileList({
     [owner, repo, number]
   );
 
-  // Measured floating-bar height (null until the first layout event).
-  const [barHeight, setBarHeight] = useState<number | null>(null);
-
-  // Stable callback: ignore sub-one-point noise to avoid unnecessary
-  // re-renders.  Layout events can fire with fractional-pixel deltas.
-  const handleHeightChange = useCallback((height: number) => {
-    setBarHeight(prev => {
-      if (prev !== null && Math.abs(prev - height) < 1) {
-        return prev;
-      }
-      return height;
-    });
-  }, []);
-
   // The write bar renders on every provider (s6): its two routes — the
   // comment composer and the review-submit sheet — are siblings of the
   // GitHub route AND of the provider route, so the bar pushes the sheet
-  // inside the scope its queries run under. The list's bottom padding
-  // reserves the bar's space at its measured (or fallback) height, so the
-  // last diff row is never hidden under it.
-  const listContentStyle = useMemo(
-    () => ({ paddingBottom: prDiffListBottomPadding(barHeight) }),
-    [barHeight]
-  );
+  // inside the scope its queries run under. The bar is an in-flow footer
+  // below the list (spot check e3), so the list only keeps a small gap
+  // between its last row and the footer's top edge.
+  const listContentStyle = useMemo(() => ({ paddingBottom: PR_DIFF_LIST_FOOTER_GAP }), []);
 
   // Which provider's words the terminal and empty states use.
   const copy = usePrDiffStateCopy({ owner, repo, number });
@@ -377,7 +363,6 @@ export function PrReviewFileList({
           viewMode={effectiveViewMode}
           selection={selection}
           onClearSelection={clearSelection}
-          onHeightChange={handleHeightChange}
         />
       </View>
     </DiffFontMetricsContext.Provider>

apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.test.tsx

@@ -310,18 +310,13 @@ describe('PrDiffFloatingActions bottom inset (plan §6)', () => {
   function findRootBar(): React.ReactElement | null {
     // eslint-disable-next-line new-cap
     const element = PrDiffFloatingActions(baseProps);
-    return findElement({
-      node: element,
-      type: 'View',
-      prop: 'onLayout',
-      value: (element.props as { onLayout?: unknown }).onLayout,
-    });
+    return element;
   }
 
   function rootPaddingBottom(): number | undefined {
     const root = findRootBar();
     if (!root) {
-      throw new Error('floating action bar root not found');
+      throw new Error('footer action bar root not found');
     }
     return (root.props as { style?: { paddingBottom?: number } }).style?.paddingBottom;
   }
@@ -335,27 +330,17 @@ describe('PrDiffFloatingActions bottom inset (plan §6)', () => {
     expect(rootPaddingBottom()).toBe(58);
   });
 
-  it('reports the measured layout height through onHeightChange', () => {
-    const onHeightChange = vi.fn(() => undefined);
-    // eslint-disable-next-line new-cap
-    const element = PrDiffFloatingActions({ ...baseProps, onHeightChange });
-    const root = findElement({
-      node: element,
-      type: 'View',
-      prop: 'onLayout',
-      value: (element.props as { onLayout?: unknown }).onLayout,
-    });
+  it('renders in-flow, not as an overlay over the list', () => {
+    // Spot check e3: the bar used to sit `absolute inset-x-0 bottom-0` over
+    // the FlashList, so a partly-scrolled diff row was clipped at its top
+    // edge. As an in-flow footer the list ends above it at every scroll
+    // position.
+    const root = findRootBar();
     if (!root) {
-      throw new Error('floating action bar root not found');
+      throw new Error('footer action bar root not found');
     }
-    const onLayout = (
-      root.props as {
-        onLayout?: (event: { nativeEvent: { layout: { height: number } } }) => void;
-      }
-    ).onLayout;
-    onLayout?.({ nativeEvent: { layout: { height: 150 } } });
-
-    expect(onHeightChange).toHaveBeenCalledTimes(1);
-    expect(onHeightChange).toHaveBeenCalledWith(150);
+    const classes = ((root.props as { className?: string }).className ?? '').split(' ');
+    expect(classes).not.toContain('absolute');
+    expect(classes).toContain('w-full');
   });
 });

apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx

@@ -1,4 +1,7 @@
-// Floating action bar rendered over the PR diff FlashList. Hosts:
+// Footer action bar rendered in-flow below the PR diff FlashList. The list
+// ends at its top edge, so a diff row is never clipped by it at any scroll
+// position (spot check e3: the bar floated over the list and cut the last
+// src/beta.ts line). Hosts:
 //   - The "Comment" affordance that pushes the comment-composer route
 //     when a diff-line selection exists, plus a "Clear" button that
 //     drops the selection.
@@ -13,7 +16,7 @@
 import { type Href, useRouter } from 'expo-router';
 import { MessageCirclePlus } from '@/components/ui/icons';
 import { useTranslation } from 'react-i18next';
-import { type LayoutChangeEvent, View } from 'react-native';
+import { View } from 'react-native';
 import { useSafeAreaInsets } from 'react-native-safe-area-context';
 
 import { Button } from '@/components/ui/button';
@@ -47,8 +50,6 @@ type PrDiffFloatingActionsProps = Readonly<{
   selection: SelectionState | null;
   /** Setter for the parent's selection state — `null` clears. */
   onClearSelection: () => void;
-  /** Optional callback for the measured root layout height (points). */
-  onHeightChange?: (height: number) => void;
 }>;
 
 export function PrDiffFloatingActions({
@@ -59,18 +60,15 @@ export function PrDiffFloatingActions({
   viewMode,
   selection,
   onClearSelection,
-  onHeightChange,
 }: PrDiffFloatingActionsProps) {
   const router = useRouter();
   const colors = useThemeColors();
   const { t } = useTranslation();
   const pending = usePendingReview();
-  // The bar sits on the bottom edge, so its bottom padding must include the
-  // Android system inset. The measured height (onLayout) therefore already
-  // includes the inset, which `prDiffListBottomPadding` reserves for the list.
-  // The container itself is opaque (`bg-background`): the card floats inside
-  // a transparent ring of padding, and without a solid backdrop the diff text
-  // of rows scrolled under the bar shows through around and below the button.
+  // The footer sits on the bottom edge, so its bottom padding must include
+  // the Android system inset. The container is opaque (`bg-background`) and
+  // in-flow: the diff list ends at its top edge, so no row is ever clipped
+  // by it and nothing shows through around the card.
   const insets = useSafeAreaInsets();
 
   const showSelectionAction = viewMode === 'unified' && selection !== null;
@@ -115,10 +113,7 @@ export function PrDiffFloatingActions({
 
   return (
     <View
-      onLayout={(event: LayoutChangeEvent) => {
-        onHeightChange?.(event.nativeEvent.layout.height);
-      }}
-      className="absolute inset-x-0 bottom-0 items-center gap-2 bg-background px-4 pt-3"
+      className="w-full items-center gap-2 bg-background px-4 pt-3"
       style={{ paddingBottom: 24 + insets.bottom }}
     >
       <View className="w-full gap-2 rounded-2xl border border-border bg-background px-3 py-3 shadow-lg shadow-[#0000001A]">

apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx

@@ -606,6 +606,37 @@ describe('PrMergeSheet provider merge arm (s6)', () => {
       findElement({ node: element, type: 'Button', prop: 'accessibilityLabel', value: 'Cancel' })
     ).not.toBeNull();
   });
+
+  it('pins the blocked-arm footer to the sheet bottom (spot check e7)', () => {
+    // eslint-disable-next-line new-cap
+    const element = PrMergeSheet({
+      ...baseProps,
+      prRef: GITLAB_REF,
+      mergeState: mergeState({
+        canMerge: false,
+        blockedReasons: [{ code: 'failing_pipeline', message: 'Pipeline #123 failed' }],
+      }),
+    });
+    // The sheet opens at the full detent; a growing spacer plus a content
+    // container that fills the viewport keep Cancel on the sheet's bottom
+    // edge instead of floating mid-sheet over an empty region.
+    const scroll = findElement({
+      node: element,
+      type: 'ScrollView',
+      prop: 'className',
+      value: 'flex-1 bg-background',
+    });
+    expect(scroll).not.toBeNull();
+    if (!scroll) {
+      return;
+    }
+    expect(
+      (scroll.props as { contentContainerStyle?: Record<string, unknown> }).contentContainerStyle
+    ).toEqual({ flexGrow: 1, paddingBottom: 4 });
+    expect(
+      findElement({ node: scroll, type: 'View', prop: 'className', value: 'flex-1' })
+    ).not.toBeNull();
+  });
 });
 
 // ── s6f: reviewer blocking findings ──────────────────────────────────

apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx

@@ -519,6 +519,16 @@ export function PrMergeSheet(props: PrMergeSheetProps) {
     onDismiss();
   }
 
+  // The short arms (blocked merge, capability banner, provider auto-merge)
+  // carry a small body; the sheet still opens at the full detent, so their
+  // footers are pinned to the sheet's bottom edge with a growing spacer
+  // (spot check e7: Cancel sat mid-sheet over a large empty region). The
+  // ScrollView's content container grows to at least the viewport, so the
+  // spacer only expands when the body is shorter than the sheet.
+  function bodySpacer() {
+    return <View className="flex-1" />;
+  }
+
   // The body a settled draft renders: the arm the provider state selects —
   // the Bitbucket auto-merge capability banner, the GitLab auto-merge body,
   // a blocked merge state's restrictions, or the form. The cancel-only
@@ -548,6 +558,7 @@ export function PrMergeSheet(props: PrMergeSheetProps) {
           <View className="gap-4 px-6 pt-4">
             <PrReviewCapabilityBanner capability={autoMergeCapability} />
           </View>
+          {bodySpacer()}
           {cancelOnlyFooter()}
         </>
       );
@@ -556,6 +567,7 @@ export function PrMergeSheet(props: PrMergeSheetProps) {
       return (
         <>
           <ProviderAutoMergeBody mergeState={mergeState} term={providerTerm} />
+          {bodySpacer()}
           <PrFormSheetFooter>
             <Button
               onPress={handleConfirmPress}
@@ -586,6 +598,7 @@ export function PrMergeSheet(props: PrMergeSheetProps) {
           <View className="gap-4 px-6 pt-4">
             <MergeRestrictionsList mergeState={mergeState} term={providerTerm} />
           </View>
+          {bodySpacer()}
           {cancelOnlyFooter()}
         </>
       );
@@ -635,7 +648,9 @@ export function PrMergeSheet(props: PrMergeSheetProps) {
       <ScrollView
         ref={scrollRef}
         className="flex-1 bg-background"
-        contentContainerClassName="pb-1"
+        // flexGrow makes the content container at least the viewport tall,
+        // so a short arm's spacer can push its footer to the sheet's bottom.
+        contentContainerStyle={{ flexGrow: 1, paddingBottom: 4 }}
         keyboardShouldPersistTaps="handled"
         automaticallyAdjustKeyboardInsets
         keyboardDismissMode="interactive"