fix(app): restore line numbers in diff gutter and add tap-to-comment
The gutter was showing the message-circle icon on every reviewable line on native and on compact form factors, displacing the line number whenever review draft mode was active. Restore the default by computing showAction purely from interaction state (hover/press), comment presence, and editor visibility — drop the showPersistentAction flag and the isNative bias. Tap on the gutter directly opens the inline review editor on every platform. On native, long-pressing a line of code (350ms) also opens the editor via a new LongPressableLine wrapper, preserving web text selection.
This commit is contained in:
@@ -6,6 +6,7 @@ import {
|
||||
useRef,
|
||||
memo,
|
||||
type ReactElement,
|
||||
type ReactNode,
|
||||
type RefObject,
|
||||
} from "react";
|
||||
import { useRouter } from "expo-router";
|
||||
@@ -230,6 +231,40 @@ const EMPTY_COMMENTS: readonly ReviewDraftComment[] = [];
|
||||
|
||||
function noopStartComment(): void {}
|
||||
|
||||
const LONG_PRESS_COMMENT_DELAY_MS = 350;
|
||||
|
||||
function LongPressableLine({
|
||||
reviewTarget,
|
||||
reviewActions,
|
||||
style,
|
||||
children,
|
||||
}: {
|
||||
reviewTarget: ReviewableDiffTarget | null | undefined;
|
||||
reviewActions: InlineReviewActions | undefined;
|
||||
style: StyleProp<ViewStyle>;
|
||||
children: ReactNode;
|
||||
}) {
|
||||
const onStartComment = reviewActions?.onStartComment;
|
||||
const handleLongPress = useCallback(() => {
|
||||
if (reviewTarget && onStartComment) {
|
||||
onStartComment(reviewTarget);
|
||||
}
|
||||
}, [reviewTarget, onStartComment]);
|
||||
|
||||
if (!isNative || !reviewTarget || !onStartComment) {
|
||||
return <View style={style}>{children}</View>;
|
||||
}
|
||||
return (
|
||||
<Pressable
|
||||
onLongPress={handleLongPress}
|
||||
delayLongPress={LONG_PRESS_COMMENT_DELAY_MS}
|
||||
style={style}
|
||||
>
|
||||
{children}
|
||||
</Pressable>
|
||||
);
|
||||
}
|
||||
|
||||
function lineTypeBackground(type: DiffLine["type"] | undefined | null) {
|
||||
if (!type) return styles.emptySplitCell;
|
||||
if (type === "add") return styles.addLineContainer;
|
||||
@@ -280,7 +315,6 @@ function DiffGutterCell({
|
||||
reviewTarget={reviewTarget}
|
||||
comments={comments}
|
||||
isEditorOpen={isEditorOpen}
|
||||
showPersistentAction={reviewActions?.showPersistentAction ?? false}
|
||||
onStartComment={onStartComment}
|
||||
style={containerStyle}
|
||||
>
|
||||
@@ -291,7 +325,17 @@ function DiffGutterCell({
|
||||
);
|
||||
}
|
||||
|
||||
function DiffTextLine({ line, wrapLines }: { line: DiffLine; wrapLines: boolean }) {
|
||||
function DiffTextLine({
|
||||
line,
|
||||
wrapLines,
|
||||
reviewTarget,
|
||||
reviewActions,
|
||||
}: {
|
||||
line: DiffLine;
|
||||
wrapLines: boolean;
|
||||
reviewTarget?: ReviewableDiffTarget | null;
|
||||
reviewActions?: InlineReviewActions;
|
||||
}) {
|
||||
const visibleTokens = hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
const containerStyle = useMemo(
|
||||
@@ -311,22 +355,28 @@ function DiffTextLine({ line, wrapLines }: { line: DiffLine; wrapLines: boolean
|
||||
);
|
||||
|
||||
return (
|
||||
<View style={containerStyle}>
|
||||
<LongPressableLine
|
||||
reviewTarget={reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
style={containerStyle}
|
||||
>
|
||||
{line.type !== "header" && visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text style={textStyle}>{formatDiffContentText(line.content)}</Text>
|
||||
)}
|
||||
</View>
|
||||
</LongPressableLine>
|
||||
);
|
||||
}
|
||||
|
||||
function SplitTextLine({
|
||||
line,
|
||||
wrapLines,
|
||||
reviewActions,
|
||||
}: {
|
||||
line: SplitDiffDisplayLine | null;
|
||||
wrapLines: boolean;
|
||||
reviewActions?: InlineReviewActions;
|
||||
}) {
|
||||
const visibleTokens = line && hasVisibleDiffTokens(line.tokens) ? line.tokens : null;
|
||||
|
||||
@@ -347,13 +397,17 @@ function SplitTextLine({
|
||||
);
|
||||
|
||||
return (
|
||||
<View style={containerStyle}>
|
||||
<LongPressableLine
|
||||
reviewTarget={line?.reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
style={containerStyle}
|
||||
>
|
||||
{visibleTokens ? (
|
||||
<HighlightedText tokens={visibleTokens} wrapLines={wrapLines} />
|
||||
) : (
|
||||
<Text style={textStyle}>{formatDiffContentText(line?.content)}</Text>
|
||||
)}
|
||||
</View>
|
||||
</LongPressableLine>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -391,7 +445,11 @@ function DiffLineView({
|
||||
);
|
||||
|
||||
return (
|
||||
<View style={containerStyle}>
|
||||
<LongPressableLine
|
||||
reviewTarget={reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
style={containerStyle}
|
||||
>
|
||||
<DiffGutterCell
|
||||
lineNumber={lineNumber}
|
||||
type={line.type}
|
||||
@@ -405,7 +463,7 @@ function DiffLineView({
|
||||
) : (
|
||||
<Text style={textStyle}>{formatDiffContentText(line.content)}</Text>
|
||||
)}
|
||||
</View>
|
||||
</LongPressableLine>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -439,7 +497,11 @@ function SplitDiffLine({
|
||||
);
|
||||
|
||||
return (
|
||||
<View style={containerStyle}>
|
||||
<LongPressableLine
|
||||
reviewTarget={line?.reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
style={containerStyle}
|
||||
>
|
||||
<DiffGutterCell
|
||||
lineNumber={line?.lineNumber ?? null}
|
||||
type={line?.type}
|
||||
@@ -453,7 +515,7 @@ function SplitDiffLine({
|
||||
) : (
|
||||
<Text style={textStyle}>{formatDiffContentText(line?.content)}</Text>
|
||||
)}
|
||||
</View>
|
||||
</LongPressableLine>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -686,7 +748,7 @@ function SplitDiffColumn({
|
||||
});
|
||||
return (
|
||||
<View key={key}>
|
||||
<SplitTextLine line={line} wrapLines={false} />
|
||||
<SplitTextLine line={line} wrapLines={false} reviewActions={reviewActions} />
|
||||
<InlineReviewThreadContent
|
||||
reviewTarget={line?.reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
@@ -931,7 +993,12 @@ function DiffFileBody({
|
||||
<View style={linesContainerRowStyle}>
|
||||
{computedLines.map(({ line, key, reviewTarget }) => (
|
||||
<View key={key}>
|
||||
<DiffTextLine line={line} wrapLines={false} />
|
||||
<DiffTextLine
|
||||
line={line}
|
||||
wrapLines={false}
|
||||
reviewTarget={reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
/>
|
||||
<InlineReviewThreadContent
|
||||
reviewTarget={reviewTarget}
|
||||
reviewActions={reviewActions}
|
||||
@@ -1872,7 +1939,6 @@ export function GitDiffPane({
|
||||
|
||||
const reviewActions = useInlineReviewController({
|
||||
reviewDraftKey,
|
||||
showPersistentAction: isMobile,
|
||||
});
|
||||
const reviewAttachment = useReviewAttachmentSnapshot({
|
||||
key: reviewDraftKey,
|
||||
|
||||
@@ -130,12 +130,12 @@ function target(overrides: Partial<ReviewableDiffTarget> = {}): ReviewableDiffTa
|
||||
}
|
||||
|
||||
const EMPTY_COMMENTS: ReviewDraftComment[] = [];
|
||||
const COMMENT_LIST: ReviewDraftComment[] = [comment()];
|
||||
|
||||
function buildReviewActions(overrides: Partial<InlineReviewActions> = {}): InlineReviewActions {
|
||||
return {
|
||||
commentsByTarget: new Map(),
|
||||
editor: null,
|
||||
showPersistentAction: false,
|
||||
onStartComment: vi.fn(),
|
||||
onEditComment: vi.fn(),
|
||||
onCancelEditor: vi.fn(),
|
||||
@@ -173,13 +173,10 @@ describe("useInlineReviewController", () => {
|
||||
const firstKey = "review:key-1";
|
||||
const secondKey = "review:key-2";
|
||||
const { result, rerender } = renderHook(
|
||||
({ reviewDraftKey }) =>
|
||||
useInlineReviewController({ reviewDraftKey, showPersistentAction: true }),
|
||||
({ reviewDraftKey }) => useInlineReviewController({ reviewDraftKey }),
|
||||
{ initialProps: { reviewDraftKey: firstKey } },
|
||||
);
|
||||
|
||||
expect(result.current.showPersistentAction).toBe(true);
|
||||
|
||||
act(() => result.current.onStartComment(reviewTarget));
|
||||
expect(result.current.editor).toEqual({ target: reviewTarget, commentId: null, body: "" });
|
||||
|
||||
@@ -243,16 +240,7 @@ describe("git diff inline review helpers", () => {
|
||||
expect(
|
||||
getInlineReviewThreadState({
|
||||
reviewTarget: target(),
|
||||
reviewActions: {
|
||||
commentsByTarget,
|
||||
editor: null,
|
||||
showPersistentAction: false,
|
||||
onStartComment: vi.fn(),
|
||||
onEditComment: vi.fn(),
|
||||
onCancelEditor: vi.fn(),
|
||||
onSaveEditor: vi.fn(),
|
||||
onDeleteComment: vi.fn(),
|
||||
},
|
||||
reviewActions: buildReviewActions({ commentsByTarget }),
|
||||
})?.comments,
|
||||
).toEqual([comments[0]]);
|
||||
});
|
||||
@@ -261,16 +249,10 @@ describe("git diff inline review helpers", () => {
|
||||
const leftTarget = target({ side: "old", lineNumber: 8, oldLineNumber: 8 });
|
||||
const rightTarget = target();
|
||||
const rightComment = comment();
|
||||
const actions: InlineReviewActions = {
|
||||
const actions = buildReviewActions({
|
||||
commentsByTarget: groupInlineReviewCommentsByTarget([rightComment]),
|
||||
editor: { target: rightTarget, commentId: null, body: "" },
|
||||
showPersistentAction: false,
|
||||
onStartComment: vi.fn(),
|
||||
onEditComment: vi.fn(),
|
||||
onCancelEditor: vi.fn(),
|
||||
onSaveEditor: vi.fn(),
|
||||
onDeleteComment: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const rowState = getSplitInlineReviewThreadState({
|
||||
left: leftTarget,
|
||||
@@ -292,7 +274,7 @@ describe("git diff inline review helpers", () => {
|
||||
).toEqual([{ position: "sticky", left: 0 }, { width: 320 }]);
|
||||
});
|
||||
|
||||
it("keeps the gutter add-comment target accessible", () => {
|
||||
it("keeps the gutter add-comment target accessible and clicking opens the editor", () => {
|
||||
const onStartComment = vi.fn();
|
||||
const reviewTarget = target();
|
||||
const { getByLabelText } = render(
|
||||
@@ -300,7 +282,6 @@ describe("git diff inline review helpers", () => {
|
||||
reviewTarget={reviewTarget}
|
||||
comments={EMPTY_COMMENTS}
|
||||
isEditorOpen={false}
|
||||
showPersistentAction={false}
|
||||
onStartComment={onStartComment}
|
||||
>
|
||||
<span>2</span>
|
||||
@@ -312,14 +293,41 @@ describe("git diff inline review helpers", () => {
|
||||
expect(pressablePropsByLabel.get("Add review comment")?.hitSlop).toBe(SMALL_ACTION_HIT_SLOP);
|
||||
});
|
||||
|
||||
it("replaces the line number with the message icon in the same gutter cell", () => {
|
||||
it("shows the line number by default and replaces it with the icon when comments exist or the editor is open", () => {
|
||||
const reviewTarget = target();
|
||||
const { container, queryByText } = render(
|
||||
const { container, queryByText, rerender } = render(
|
||||
<InlineReviewGutterCell
|
||||
reviewTarget={reviewTarget}
|
||||
comments={EMPTY_COMMENTS}
|
||||
isEditorOpen={false}
|
||||
showPersistentAction
|
||||
onStartComment={vi.fn()}
|
||||
>
|
||||
<span>2</span>
|
||||
</InlineReviewGutterCell>,
|
||||
);
|
||||
|
||||
expect(queryByText("2")).toBeTruthy();
|
||||
expect(container.querySelector("[data-icon='MessageCircle']")).toBeNull();
|
||||
|
||||
rerender(
|
||||
<InlineReviewGutterCell
|
||||
reviewTarget={reviewTarget}
|
||||
comments={COMMENT_LIST}
|
||||
isEditorOpen={false}
|
||||
onStartComment={vi.fn()}
|
||||
>
|
||||
<span>2</span>
|
||||
</InlineReviewGutterCell>,
|
||||
);
|
||||
|
||||
expect(queryByText("2")).toBeNull();
|
||||
expect(container.querySelector("[data-icon='MessageCircle']")).toBeTruthy();
|
||||
|
||||
rerender(
|
||||
<InlineReviewGutterCell
|
||||
reviewTarget={reviewTarget}
|
||||
comments={EMPTY_COMMENTS}
|
||||
isEditorOpen
|
||||
onStartComment={vi.fn()}
|
||||
>
|
||||
<span>2</span>
|
||||
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
type ViewStyle,
|
||||
} from "react-native";
|
||||
import { StyleSheet, useUnistyles } from "react-native-unistyles";
|
||||
import { isNative, isWeb } from "@/constants/platform";
|
||||
import { isWeb } from "@/constants/platform";
|
||||
import { useReviewDraftComments, useReviewDraftStore, type ReviewDraftComment } from "./store";
|
||||
import { buildReviewableDiffTargetKey, type ReviewableDiffTarget } from "@/utils/diff-layout";
|
||||
|
||||
@@ -44,7 +44,6 @@ export interface InlineReviewEditorState {
|
||||
export interface InlineReviewActions {
|
||||
commentsByTarget: ReadonlyMap<string, ReviewDraftComment[]>;
|
||||
editor: InlineReviewEditorState | null;
|
||||
showPersistentAction: boolean;
|
||||
onStartComment: (target: ReviewableDiffTarget) => void;
|
||||
onEditComment: (target: ReviewableDiffTarget, comment: ReviewDraftComment) => void;
|
||||
onCancelEditor: () => void;
|
||||
@@ -63,10 +62,7 @@ export function groupInlineReviewCommentsByTarget(
|
||||
return grouped;
|
||||
}
|
||||
|
||||
export function useInlineReviewController(input: {
|
||||
reviewDraftKey: string;
|
||||
showPersistentAction: boolean;
|
||||
}): InlineReviewActions {
|
||||
export function useInlineReviewController(input: { reviewDraftKey: string }): InlineReviewActions {
|
||||
const reviewComments = useReviewDraftComments(input.reviewDraftKey);
|
||||
const commentsByTarget = useMemo(
|
||||
() => groupInlineReviewCommentsByTarget(reviewComments),
|
||||
@@ -137,7 +133,6 @@ export function useInlineReviewController(input: {
|
||||
() => ({
|
||||
commentsByTarget,
|
||||
editor,
|
||||
showPersistentAction: input.showPersistentAction,
|
||||
onStartComment: handleStartComment,
|
||||
onEditComment: handleEditComment,
|
||||
onCancelEditor: handleCancelEditor,
|
||||
@@ -152,7 +147,6 @@ export function useInlineReviewController(input: {
|
||||
handleEditComment,
|
||||
handleSaveEditor,
|
||||
handleStartComment,
|
||||
input.showPersistentAction,
|
||||
],
|
||||
);
|
||||
}
|
||||
@@ -235,7 +229,6 @@ export function InlineReviewGutterCell({
|
||||
reviewTarget,
|
||||
comments,
|
||||
isEditorOpen,
|
||||
showPersistentAction,
|
||||
onStartComment,
|
||||
style,
|
||||
}: {
|
||||
@@ -243,7 +236,6 @@ export function InlineReviewGutterCell({
|
||||
reviewTarget: ReviewableDiffTarget | null | undefined;
|
||||
comments: readonly ReviewDraftComment[];
|
||||
isEditorOpen: boolean;
|
||||
showPersistentAction: boolean;
|
||||
onStartComment: (target: ReviewableDiffTarget) => void;
|
||||
style?: StyleProp<ViewStyle>;
|
||||
}) {
|
||||
@@ -280,9 +272,7 @@ export function InlineReviewGutterCell({
|
||||
style={pressableStyle}
|
||||
>
|
||||
{({ hovered, pressed }) => {
|
||||
const showAction =
|
||||
canComment &&
|
||||
(hovered || pressed || isNative || showPersistentAction || hasComments || isEditorOpen);
|
||||
const showAction = canComment && (hovered || pressed || hasComments || isEditorOpen);
|
||||
return (
|
||||
<View style={styles.gutterInner}>
|
||||
<View style={labelStyle}>
|
||||
|
||||
Reference in New Issue
Block a user