diff --git a/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts b/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts index 39a560b99..546bc65e2 100644 --- a/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts +++ b/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts @@ -3121,7 +3121,7 @@ const messageHandlers: Record Promise { const typedEvent = event as Extract; - const { currentCellId, previousCellId, currentContent, previousContent, message } = typedEvent.content; + const { currentCellId, previousCellId, currentContent, previousContent } = typedEvent.content; debug("confirmCellMerge message received for cells:", { currentCellId, previousCellId }); @@ -3149,45 +3149,36 @@ const messageHandlers: Record Promise { - provider.refreshWebview(webviewPanel, document); - } - }); + // Call the existing merge handler + await messageHandlers.mergeCellWithPrevious({ + event: mergeEvent, + document, + webviewPanel, + provider, + updateWebview: () => { + provider.refreshWebview(webviewPanel, document); + } + }); - // Only merge in target if we're working with a source file - if (isSourceFile) { - const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); - if (!workspaceFolder) { - throw new Error("No workspace folder found"); - } - await provider.mergeMatchingCellsInTargetFile(currentCellId, previousCellId, document.uri.toString(), workspaceFolder); + // Only merge in target if we're working with a source file + if (isSourceFile) { + const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + if (!workspaceFolder) { + throw new Error("No workspace folder found"); } + await provider.mergeMatchingCellsInTargetFile(currentCellId, previousCellId, document.uri.toString(), workspaceFolder); } } catch (error) { console.error("Error in confirmCellMerge:", error); diff --git a/types/index.d.ts b/types/index.d.ts index 4a62f3622..0221bb449 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -603,7 +603,6 @@ export type EditorPostMessages = previousCellId: string; currentContent: string; previousContent: string; - message: string; }; } | { diff --git a/webviews/codex-webviews/src/CodexCellEditor/CellContentDisplay.tsx b/webviews/codex-webviews/src/CodexCellEditor/CellContentDisplay.tsx index 7529e89ac..a32ed2630 100644 --- a/webviews/codex-webviews/src/CodexCellEditor/CellContentDisplay.tsx +++ b/webviews/codex-webviews/src/CodexCellEditor/CellContentDisplay.tsx @@ -137,6 +137,13 @@ const CellContentDisplay: React.FC = React.memo( const [showSparkleButton, setShowSparkleButton] = useState(false); const [showAuthModal, setShowAuthModal] = useState(false); const [showOfflineModal, setShowOfflineModal] = useState(false); + // Pending merge awaiting in-editor confirmation (replaces the native VS Code popup) + const [pendingMerge, setPendingMerge] = useState<{ + currentCellId: string; + previousCellId: string; + currentContent: string; + previousContent: string; + } | null>(null); const [isLockButtonGlowing, setIsLockButtonGlowing] = useState(false); const [isLockButtonFlashing, setIsLockButtonFlashing] = useState(false); const [isResolvingStructure, setIsResolvingStructure] = useState(false); @@ -390,18 +397,27 @@ const CellContentDisplay: React.FC = React.memo( return; } - // Send confirmation request to VS Code instead of using window.confirm + // Open an in-editor confirmation modal instead of the native VS Code popup + // (which appears in the bottom-right corner of the screen, far from the cell). + setPendingMerge({ + currentCellId: currentCell.cellMarkers[0], + previousCellId: targetCell.cellMarkers[0], + currentContent: currentCell.cellContent, + previousContent: targetCell.cellContent, + }); + }; + + const handleConfirmMerge = () => { + if (!pendingMerge) return; vscode.postMessage({ command: "confirmCellMerge", - content: { - currentCellId: currentCell.cellMarkers[0], - previousCellId: targetCell.cellMarkers[0], - currentContent: currentCell.cellContent, - previousContent: targetCell.cellContent, - message: - "Are you sure you want to merge this cell with the previous non-merged cell? This action cannot be undone.", - }, + content: pendingMerge, } as any); + setPendingMerge(null); + }; + + const handleCancelMergeConfirmation = () => { + setPendingMerge(null); }; // Line numbers are always generated and shown at the beginning of each line @@ -977,6 +993,34 @@ const CellContentDisplay: React.FC = React.memo( )} + { + if (!open) handleCancelMergeConfirmation(); + }} + > + + + Merge with previous cell + + This cell will be merged into the previous + non-merged cell. You can reverse this later by + unmerging the cell. + + + + + + + + )} diff --git a/webviews/codex-webviews/src/CodexCellEditor/CellList.tsx b/webviews/codex-webviews/src/CodexCellEditor/CellList.tsx index d7a46e011..ad03f51b4 100644 --- a/webviews/codex-webviews/src/CodexCellEditor/CellList.tsx +++ b/webviews/codex-webviews/src/CodexCellEditor/CellList.tsx @@ -157,24 +157,21 @@ const CellList: React.FC = ({ // State to track unresolved comments count for each cell const [cellCommentsCount, setCellCommentsCount] = useState>(new Map()); - // Filter out merged cells if we're in correction editor mode for source text const filteredTranslationUnits = useMemo(() => { let filtered = translationUnits; - // Filter out merged cells if we're in correction editor mode for source text - if (isSourceText && isCorrectionEditorMode) { - filtered = filtered.filter((unit) => { - // Check if cell has merged metadata in the data property - const cellData = unit.data as any; - return !cellData?.merged; - }); - } + // NOTE: Do NOT filter out merged cells here. In source + correction editor mode the + // provider intentionally keeps merged cells in the translation units (see + // mergeRangesAndProcess) so they render greyed-out with the "unmerge" button. In every + // other mode the provider already strips merged cells, so they never reach this list. + // Filtering them here previously hid them in correction mode, which removed the only + // place the unmerge button could appear (issue #691). // Filter out milestone cells from the view (they remain in JSON) filtered = filtered.filter((unit) => unit.cellType !== CodexCellTypes.MILESTONE); return filtered; - }, [translationUnits, isSourceText, isCorrectionEditorMode]); + }, [translationUnits]); // Use filtered units for all operations const workingTranslationUnits = filteredTranslationUnits; // State to track completed translations (only successful ones) - REMOVED: Now handled by parent diff --git a/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellContentDisplay.mergeConfirm.test.tsx b/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellContentDisplay.mergeConfirm.test.tsx new file mode 100644 index 000000000..8af051c59 --- /dev/null +++ b/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellContentDisplay.mergeConfirm.test.tsx @@ -0,0 +1,173 @@ +import React from "react"; +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { QuillCellContent } from "../../../../../types"; +import { CodexCellTypes } from "../../../../../types/enums"; +import CellContentDisplay from "../CellContentDisplay"; + +// Regression test for the #691 follow-up: the merge confirmation is now an in-editor modal +// (rendered inside the source editor) instead of a native VS Code popup in the screen corner. +// Clicking the merge button must open the modal and NOT post `confirmCellMerge` until the user +// confirms in the modal. + +const mockVscode = { + postMessage: vi.fn(), + getState: vi.fn(), + setState: vi.fn(), +}; + +Object.defineProperty(window, "vscodeApi", { + value: mockVscode, + writable: true, +}); + +global.acquireVsCodeApi = vi.fn().mockReturnValue(mockVscode); + +vi.mock("@sharedUtils", () => ({ + shouldDisableValidation: vi.fn().mockReturnValue(false), +})); + +vi.mock("../contextProviders/UnsavedChangesContext", () => ({ + default: React.createContext({ + setUnsavedChanges: vi.fn(), + showFlashingBorder: false, + unsavedChanges: false, + toggleFlashingBorder: vi.fn(), + }), +})); + +vi.mock("../contextProviders/TooltipContext", () => ({ + useTooltip: () => ({ + showTooltip: vi.fn(), + hideTooltip: vi.fn(), + }), +})); + +vi.mock("../hooks/useCentralizedMessageDispatcher", () => ({ + useMessageHandler: vi.fn(() => {}), +})); + +vi.mock("../lib/audioController", () => ({ + globalAudioController: { + playExclusive: vi.fn().mockResolvedValue(undefined), + addListener: vi.fn(), + removeListener: vi.fn(), + }, +})); + +vi.mock("../lib/audioCache", () => ({ + getCachedAudioDataUrl: vi.fn().mockReturnValue(null), + setCachedAudioDataUrl: vi.fn(), +})); + +vi.mock("../ValidationButton", () => ({ + default: () =>
, +})); + +vi.mock("../AudioValidationButton", () => ({ + default: () => ( +
+ ), +})); + +vi.mock("../CommentsBadge", () => ({ + default: () =>
, +})); + +vi.mock("react-markdown", () => ({ + default: ({ children }: { children: React.ReactNode }) =>
{children}
, +})); + +const createMockCell = (cellId: string, content: string): QuillCellContent => ({ + cellMarkers: [cellId], + cellContent: content, + cellType: CodexCellTypes.TEXT, + editHistory: [ + { + editMap: ["value"], + value: content, + author: "test-user", + validatedBy: [], + timestamp: Date.now(), + type: "user-edit" as any, + }, + ], + cellLabel: cellId, + timestamps: { startTime: 0, endTime: 5 }, +}); + +const renderSecondCell = () => { + const firstCell = createMockCell("cell-1", "

First

"); + const secondCell = createMockCell("cell-2", "

Second

"); + const props = { + cell: secondCell, + vscode: mockVscode as any, + textDirection: "ltr" as const, + isSourceText: true, + hasDuplicateId: false, + highlightedCellId: null, + scrollSyncEnabled: true, + lineNumber: "2", + label: "Test Label", + lineNumbersEnabled: true, + isInTranslationProcess: false, + translationState: null as any, + allTranslationsComplete: false, + handleCellClick: vi.fn(), + audioAttachments: { "cell-2": "available" as const }, + currentUsername: "test-user", + requiredValidations: 1, + requiredAudioValidations: 1, + isCorrectionEditorMode: true, + translationUnits: [firstCell, secondCell], + }; + return render(); +}; + +const clickMergeButton = (container: HTMLElement) => { + const mergeButton = container.querySelector(".codicon-merge")?.closest("button"); + expect(mergeButton).toBeTruthy(); + fireEvent.click(mergeButton!); +}; + +const confirmCellMergeCalls = () => + mockVscode.postMessage.mock.calls.filter(([msg]) => msg?.command === "confirmCellMerge"); + +describe("CellContentDisplay - merge confirmation modal (#691 follow-up)", () => { + beforeEach(() => { + vi.clearAllMocks(); + Element.prototype.scrollIntoView = vi.fn(); + }); + + it("opens the in-editor modal on merge click and does not post confirmCellMerge yet", () => { + const { container } = renderSecondCell(); + clickMergeButton(container); + + // Modal is shown... + expect(screen.getByText("Merge with previous cell")).toBeTruthy(); + // ...but nothing is committed until the user confirms. + expect(confirmCellMergeCalls()).toHaveLength(0); + }); + + it("posts confirmCellMerge (without a message field) when the merge is confirmed", () => { + const { container } = renderSecondCell(); + clickMergeButton(container); + fireEvent.click(screen.getByRole("button", { name: "Merge" })); + + const calls = confirmCellMergeCalls(); + expect(calls).toHaveLength(1); + const msg = calls[0][0]; + expect(msg.content.currentCellId).toBe("cell-2"); + expect(msg.content.previousCellId).toBe("cell-1"); + expect(msg.content).not.toHaveProperty("message"); + }); + + it("does not post confirmCellMerge when the modal is cancelled", () => { + const { container } = renderSecondCell(); + clickMergeButton(container); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + + expect(confirmCellMergeCalls()).toHaveLength(0); + expect(screen.queryByText("Merge with previous cell")).toBeNull(); + }); +}); diff --git a/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellList.mergedCellUnmerge.test.tsx b/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellList.mergedCellUnmerge.test.tsx new file mode 100644 index 000000000..6ac68c348 --- /dev/null +++ b/webviews/codex-webviews/src/CodexCellEditor/__tests___/CellList.mergedCellUnmerge.test.tsx @@ -0,0 +1,178 @@ +import React from "react"; +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { render } from "@testing-library/react"; +import { QuillCellContent } from "../../../../../types"; +import { CodexCellTypes } from "../../../../../types/enums"; +import CellList from "../CellList"; + +// Regression test for issue #691: "merged content no longer shows the option to unmerge". +// +// In source + correction-editor mode the provider intentionally keeps merged cells in the +// translation units so they render greyed-out with an "unmerge" (Cancel merge) button. +// A filter in CellList used to strip merged cells in exactly that mode, which removed the +// only place the unmerge button could appear. This test locks in that merged cells survive +// to render and expose the unmerge control. + +// Mock the VSCode API +const mockVscode = { + postMessage: vi.fn(), + getState: vi.fn(), + setState: vi.fn(), +}; + +Object.defineProperty(window, "vscodeApi", { + value: mockVscode, + writable: true, +}); + +global.acquireVsCodeApi = vi.fn().mockReturnValue(mockVscode); + +vi.mock("@sharedUtils", () => ({ + shouldDisableValidation: vi.fn().mockReturnValue(false), +})); + +vi.mock("../contextProviders/UnsavedChangesContext", () => ({ + default: React.createContext({ + setUnsavedChanges: vi.fn(), + showFlashingBorder: false, + unsavedChanges: false, + toggleFlashingBorder: vi.fn(), + }), +})); + +vi.mock("../contextProviders/TooltipContext", () => ({ + useTooltip: () => ({ + showTooltip: vi.fn(), + hideTooltip: vi.fn(), + }), +})); + +vi.mock("../hooks/useCentralizedMessageDispatcher", () => ({ + useMessageHandler: vi.fn(() => {}), +})); + +vi.mock("../lib/audioController", () => ({ + globalAudioController: { + playExclusive: vi.fn().mockResolvedValue(undefined), + addListener: vi.fn(), + removeListener: vi.fn(), + }, +})); + +vi.mock("../lib/audioCache", () => ({ + getCachedAudioDataUrl: vi.fn().mockReturnValue(null), + setCachedAudioDataUrl: vi.fn(), +})); + +vi.mock("../ValidationButton", () => ({ + default: () =>
, +})); + +vi.mock("../AudioValidationButton", () => ({ + default: () => ( +
+ ), +})); + +vi.mock("../CommentsBadge", () => ({ + default: () =>
, +})); + +vi.mock("react-markdown", () => ({ + default: ({ children }: { children: React.ReactNode }) =>
{children}
, +})); + +const createCell = ( + uuid: string, + content: string, + overrides: Partial = {} +): QuillCellContent => ({ + cellMarkers: [uuid], + cellContent: content, + cellType: CodexCellTypes.TEXT, + data: {}, + editHistory: [ + { + editMap: ["value"], + value: content, + author: "test-user", + validatedBy: [], + timestamp: Date.now(), + type: "user-edit" as any, + }, + ], + cellLabel: uuid, + timestamps: { startTime: 0, endTime: 5 }, + ...overrides, +}); + +const baseProps = (translationUnits: QuillCellContent[], isCorrectionEditorMode: boolean) => ({ + translationUnits, + fullDocumentTranslationUnits: translationUnits, + contentBeingUpdated: { + cellMarkers: [], + cellContent: "", + cellChanged: false, + }, + setContentBeingUpdated: vi.fn(), + handleCloseEditor: vi.fn(), + handleSaveHtml: vi.fn(), + vscode: mockVscode, + textDirection: "ltr" as const, + isSourceText: true, + isCorrectionEditorMode, + windowHeight: 800, + headerHeight: 100, + highlightedCellId: null, + scrollSyncEnabled: true, + currentUsername: "test-user", + requiredValidations: 1, + milestoneIndex: null, + currentMilestoneIndex: 0, + currentSubsectionIndex: 0, + cellsPerPage: 50, +}); + +describe("CellList - merged cell unmerge button (issue #691)", () => { + beforeEach(() => { + vi.clearAllMocks(); + Element.prototype.scrollIntoView = vi.fn(); + }); + + it("renders a merged cell with the unmerge button in source + correction editor mode", () => { + const translationUnits: QuillCellContent[] = [ + createCell("uuid-1", "

First cell

"), + createCell("uuid-2", "

Merged cell

", { + merged: true, + data: { merged: true }, + }), + ]; + + const { container } = render( + + ); + + // The merged cell content must still render (not filtered out)... + expect(container.textContent).toContain("Merged cell"); + // ...and it must expose the unmerge ("Cancel merge") control. + expect(container.querySelector('[title="Cancel merge"]')).not.toBeNull(); + }); + + it("does not show the unmerge button when not in correction editor mode", () => { + // Outside correction mode the provider never sends merged cells, but even if one slipped + // through, the unmerge button should only appear in correction mode. + const translationUnits: QuillCellContent[] = [ + createCell("uuid-1", "

First cell

"), + createCell("uuid-2", "

Merged cell

", { + merged: true, + data: { merged: true }, + }), + ]; + + const { container } = render( + + ); + + expect(container.querySelector('[title="Cancel merge"]')).toBeNull(); + }); +});