diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index e0bbe074..d3f5fb54 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -165,6 +165,7 @@ "%interlinearizer_error_pt9Import_failed%": "Could not import the Paratext 9 interlinear data. Please try again.", "%interlinearizer_warning_pt9Import_sourceEmpty%": "The project's Paratext 9 interlinear files are missing, so the imported data was left as it was.", "%interlinearizer_error_createEditableCopy_failed%": "Could not copy the imported project. Please try again.", + "%interlinearizer_error_pt9Import_load_failed%": "The imported interlinear data could not be loaded. Try syncing from Paratext 9.", "%interlinearizer_modal_select_importPt9%": "Import from Paratext 9", "%interlinearizer_readonly_chip%": "Read-only", diff --git a/src/__tests__/analysis-store-read-only-mock.ts b/src/__tests__/analysis-store-read-only-mock.ts new file mode 100644 index 00000000..96b7a55a --- /dev/null +++ b/src/__tests__/analysis-store-read-only-mock.ts @@ -0,0 +1,31 @@ +/** + * Shared access to the manual `AnalysisStore` mock's read-only switch, for the several test files + * that render components under `useAnalysisReadOnly`. + * + * This lives apart from `test-helpers` on purpose: that module imports the real + * `AnalysisStoreProvider`, so in a file that mocks `AnalysisStore` its `withAnalysisStore` would + * silently render the mock's provider instead. + */ + +/** The manual AnalysisStore mock's test-only controls. */ +interface AnalysisStoreReadOnlyMock { + __setMockAnalysisReadOnly: (value: boolean) => void; +} + +function isAnalysisStoreReadOnlyMock(m: unknown): m is AnalysisStoreReadOnlyMock { + return !!m && typeof m === 'object' && '__setMockAnalysisReadOnly' in m; +} + +/** + * Sets what the mocked `useAnalysisReadOnly` returns. Resolves the mock on each call rather than at + * import time, so importing this module never depends on `jest.mock` having run first. + * + * @param value Whether the mocked store reports the analysis as read-only. + */ +export function setMockAnalysisReadOnly(value: boolean): void { + const analysisStoreMock: unknown = jest.requireMock('../components/AnalysisStore'); + if (!isAnalysisStoreReadOnlyMock(analysisStoreMock)) + throw new Error('Expected the AnalysisStore manual mock with read-only controls'); + const { __setMockAnalysisReadOnly: setReadOnly } = analysisStoreMock; + setReadOnly(value); +} diff --git a/src/__tests__/components/ArcOverlay.test.tsx b/src/__tests__/components/ArcOverlay.test.tsx index d3acd4bc..8e16a5bf 100644 --- a/src/__tests__/components/ArcOverlay.test.tsx +++ b/src/__tests__/components/ArcOverlay.test.tsx @@ -5,9 +5,16 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { ArcOverlay } from '../../components/ArcOverlay'; import type { ArcPath } from '../../utils/phrase-arc'; +import { setMockAnalysisReadOnly } from '../analysis-store-read-only-mock'; import { makePhraseLink } from '../test-helpers'; import { withTooltipProvider } from './test-helpers'; +jest.mock('../../components/AnalysisStore'); + +beforeEach(() => { + setMockAnalysisReadOnly(false); +}); + /** Builds a minimal `ArcPath` fixture. */ function makeArcPath(phraseId: string, splitAfterTokenRef = 'tok-a'): ArcPath { // `d` is derived from splitAfterTokenRef so distinct split points yield distinct @@ -45,7 +52,13 @@ function requiredProps(): Parameters[0] { * require. */ function renderOverlay(overrides: Partial[0]> = {}) { - return render(withTooltipProvider()); + const props = { ...requiredProps(), ...overrides }; + const result = render(withTooltipProvider()); + return { + ...result, + /** Re-renders with the same props, for a test that changed what the mocked store reports. */ + rerenderOverlay: () => result.rerender(withTooltipProvider()), + }; } describe('ArcOverlay', () => { @@ -75,6 +88,17 @@ describe('ArcOverlay', () => { expect(screen.queryByTestId('split-arc-btn')).not.toBeInTheDocument(); }); + it('draws the arcs but no split buttons for a read-only analysis', () => { + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + setMockAnalysisReadOnly(true); + renderOverlay({ + arcPaths: [makeArcPath('p1', 'tok-a')], + phraseLinkById: new Map([['p1', phraseLink]]), + }); + expect(document.querySelectorAll('path')).toHaveLength(1); + expect(screen.queryByTestId('split-arc-btn')).not.toBeInTheDocument(); + }); + it('renders a split button in view mode even when the arc phrase is neither hovered nor focused', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); renderOverlay({ @@ -224,6 +248,56 @@ describe('ArcOverlay', () => { expect(onSplitHoverChange).toHaveBeenLastCalledWith(new Set()); }); + it('clears the freed-token preview when the analysis turns read-only mid-hover', async () => { + const onSplitHoverChange = jest.fn(); + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + const { rerenderOverlay } = renderOverlay({ + arcPaths: [makeArcPath('p1', 'tok-a')], + hoveredPhraseId: 'p1', + phraseLinkById: new Map([['p1', phraseLink]]), + tokenDocOrder: new Map([ + ['tok-a', 0], + ['tok-b', 1], + ]), + onSplitHoverChange, + }); + await userEvent.hover(screen.getByTestId('split-arc-btn')); + expect(onSplitHoverChange).toHaveBeenLastCalledWith(new Set(['tok-a', 'tok-b'])); + + // The button vanishes with the mouse still over it, so no mouse-leave of its own ever fires. + setMockAnalysisReadOnly(true); + rerenderOverlay(); + + expect(screen.queryByTestId('split-arc-btn')).not.toBeInTheDocument(); + expect(onSplitHoverChange).toHaveBeenLastCalledWith(new Set()); + }); + + it('clears the phrase highlight when the analysis turns read-only mid-reshape-hover', async () => { + const onHoverPhrase = jest.fn(); + // Four-token phrase: splitting after tok-b leaves both halves ≥ 2, the reshape preview. + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); + const { rerenderOverlay } = renderOverlay({ + arcPaths: [makeArcPath('p1', 'tok-b')], + hoveredPhraseId: 'p1', + phraseLinkById: new Map([['p1', phraseLink]]), + tokenDocOrder: new Map([ + ['tok-a', 0], + ['tok-b', 1], + ['tok-c', 2], + ['tok-d', 3], + ]), + onHoverPhrase, + }); + await userEvent.hover(screen.getByTestId('split-arc-btn')); + expect(onHoverPhrase).toHaveBeenLastCalledWith('p1'); + + setMockAnalysisReadOnly(true); + rerenderOverlay(); + + expect(screen.queryByTestId('split-arc-btn')).not.toBeInTheDocument(); + expect(onHoverPhrase).toHaveBeenLastCalledWith(undefined); + }); + it('does not call onSplitHoverChange with free refs on enter when no token would become free (both halves ≥ 2)', async () => { const onSplitHoverChange = jest.fn(); // Four-token phrase: splitting after tok-b gives before=[tok-a,tok-b] and after=[tok-c,tok-d], both ≥ 2. diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index a624945f..0db88d63 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -101,9 +101,12 @@ const mockPhraseLinkById = new Map(); /** Read once per `Interlinearizer` render, so this doubles as a render counter. */ let phraseLinkByIdMapReads = 0; +/** What the mocked `useAnalysisReadOnly` reports; reset in `beforeEach`. */ +let mockReadOnly = false; + jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, - useAnalysisReadOnly: () => false, + useAnalysisReadOnly: () => mockReadOnly, /** * Pass-through provider stub that renders children directly, keeping AnalysisStore.tsx out of * scope. @@ -424,6 +427,7 @@ beforeEach(() => { // The phrase-link map is a plain Map (not a jest mock), so resetMocks does not clear it. mockPhraseLinkById.clear(); capturedSegmentation = undefined; + mockReadOnly = false; // The merge control's label comes from a localized string. mockKeyAsValueLocalizedStrings(); }); @@ -1641,6 +1645,14 @@ describe('between-rows merge control', () => { expect(button).toHaveAttribute('title', 'Merge'); }); + it('renders no merge control for a read-only analysis', () => { + // Omitted rather than disabled: a read-only analysis offers no boundary editing at all. + mockReadOnly = true; + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + expect(screen.queryByTestId('segment-merge-btn')).not.toBeInTheDocument(); + expect(screen.queryByTestId('segment-merge-indicator')).not.toBeInTheDocument(); + }); + it('renders no merge control while a phrase mode is active', () => { // A merge mid-mode could re-segment the phrase the mode UI is operating on, so the between-rows // control is omitted entirely (not merely disabled) throughout a phrase edit. diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 5bebca51..041c013d 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -4,7 +4,7 @@ import papi, { logger } from '@papi/frontend'; import { useData, useLocalizedStrings, useSetting } from '@papi/frontend/react'; import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Book, DraftProject, PhraseAnalysisLink, TextAnalysis } from 'interlinearizer'; import type { Dispatch, ReactNode, SetStateAction } from 'react'; @@ -15,6 +15,7 @@ import { RECENTER_FADE_MS } from '../../components/recenter-fade'; import useInterlinearizerBookData from '../../hooks/useInterlinearizerBookData'; import useOptimisticBooleanSetting from '../../hooks/useOptimisticBooleanSetting'; import { emptyAnalysis, emptyDraft } from '../../types/empty-factories'; +import { PT9_MANIFEST_TIMEOUT_MS } from '../../utils/pt9-manifest'; import type { PhraseMode } from '../../types/phrase-mode'; import type { ViewOptions } from '../../types/view-options'; import type { SegmentationDispatch } from '../../components/SegmentationStore'; @@ -1180,7 +1181,7 @@ describe('InterlinearizerLoader', () => { ); }); - it('returns to the plain view when an offer-run report is closed', async () => { + it('offers an offer-run report no way out but opening the import it just made', async () => { mockOfferProbe(); mockImportCommands({ importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT }, @@ -1193,11 +1194,15 @@ describe('InterlinearizerLoader', () => { ); await screen.findByTestId('pt9-import-report'); + expect( + screen.queryByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }), + ).not.toBeInTheDocument(); + await userEvent.click( - screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }), + screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_open%' }), ); - expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'none'); + expect(await screen.findByTestId('pt9-import-banner')).toBeInTheDocument(); }); it('persists the empty draft and runs no import on No', async () => { @@ -1447,6 +1452,60 @@ describe('InterlinearizerLoader', () => { ); }); + it('titles the open-path sync as a sync while it runs', async () => { + mockSendCommand.mockImplementation(async (...args) => { + if (args[0] === 'interlinearizer.importPt9Project') return new Promise(() => {}); + return JSON.stringify(emptyDraft(testProjectId)); + }); + mockPdpGet.mockResolvedValue({ + getPt9InterlinearManifest: async () => ({ 'Lexicon.xml': 'bbbb2222' }), + }); + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + await userEvent.click(screen.getByTestId('select-modal-open-import')); + + expect(await screen.findByTestId('pt9-import-running')).toHaveTextContent( + '%interlinearizer_pt9ImportModal_syncing%', + ); + }); + + it('opens the stored import with a warning when the manifest read on open never answers', async () => { + // The select modal is held inert for the whole open, so the wait has to end for the user to + // get out of it. + jest.useFakeTimers(); + try { + mockImportCommands(); + jest.mocked(papi.notifications.send).mockResolvedValue('notification-id'); + mockPdpGet.mockResolvedValue({ + getPt9InterlinearManifest: jest.fn(() => new Promise(() => {})), + }); + await act(async () => { + renderLoader(); + }); + fireEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + fireEvent.click(screen.getByTestId('select-modal-open-import')); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'select'); + + await act(async () => { + jest.advanceTimersByTime(PT9_MANIFEST_TIMEOUT_MS); + }); + + expect(jest.mocked(papi.notifications.send)).toHaveBeenCalledWith({ + message: '%interlinearizer_warning_pt9Sync_failed%', + severity: 'warning', + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'none'); + expect(screen.getByTestId('project-modals')).toHaveAttribute( + 'data-active-project-name', + 'Paratext 9 Interlinear', + ); + } finally { + jest.useRealTimers(); + } + }); + it('opens the stored import with a warning when the open-path sync fails', async () => { mockImportCommands(); mockPdpGet.mockRejectedValue(new Error('provider unavailable')); @@ -1545,6 +1604,133 @@ describe('InterlinearizerLoader', () => { ); }); + it('reseeds the view with the analysis a sync fetched, not the one it replaced', async () => { + const syncedAnalysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-synced', surfaceText: 'In', gloss: { en: 'in' } }, + ], + }; + let synced = false; + mockSendCommand.mockImplementation(async (...args) => { + if (args[0] === 'interlinearizer.getProject') + return JSON.stringify( + synced + ? { ...FRESH_IMPORT_SUMMARY, analysis: syncedAnalysis } + : { ...STUB_IMPORT_PROJECT, analysis: emptyAnalysis() }, + ); + if (args[0] === 'interlinearizer.importPt9Project') { + synced = true; + return JSON.stringify({ + outcome: 'imported', + projectId: 'import-1', + report: IMPORT_REPORT, + }); + } + return JSON.stringify(emptyDraft(testProjectId)); + }); + await renderImportView(); + expect(capturedStoreProps?.initialAnalysis).toEqual(emptyAnalysis()); + + await userEvent.click(screen.getByTestId('pt9-sync-button')); + + await waitFor(() => expect(capturedStoreProps?.initialAnalysis).toEqual(syncedAnalysis)); + }); + + it('empties the import view when a refresh brings back no analysis, rather than keeping the pre-sync one', async () => { + let synced = false; + mockSendCommand.mockImplementation(async (...args) => { + // Once synced, the record comes back as a valid summary carrying no analysis at all: the + // sync's own summary fetch is satisfied, while the refresh behind it finds nothing to show. + if (args[0] === 'interlinearizer.getProject') + return JSON.stringify( + synced ? FRESH_IMPORT_SUMMARY : { ...STUB_IMPORT_PROJECT, analysis: emptyAnalysis() }, + ); + if (args[0] === 'interlinearizer.importPt9Project') { + synced = true; + return JSON.stringify({ + outcome: 'imported', + projectId: 'import-1', + report: IMPORT_REPORT, + }); + } + return JSON.stringify(emptyDraft(testProjectId)); + }); + await renderImportView(); + expect(screen.getByTestId('interlinearizer')).toBeInTheDocument(); + + await userEvent.click(screen.getByTestId('pt9-sync-button')); + + await waitFor(() => expect(screen.queryByTestId('interlinearizer')).not.toBeInTheDocument()); + expect(screen.getByTestId('pt9-import-load-error')).toHaveTextContent( + '%interlinearizer_error_pt9Import_load_failed%', + ); + expect(screen.getByTestId('pt9-import-banner')).toBeInTheDocument(); + // The panel line is the whole message: no toast doubles it with different advice. + expect(jest.mocked(papi.notifications.send)).not.toHaveBeenCalledWith({ + message: '%interlinearizer_error_load_projects_failed%', + severity: 'error', + }); + }); + + it('hands the import view a segmentation dispatch that cannot write the draft', async () => { + mockImportCommands(); + await renderImportView(); + mockSendCommand.mockClear(); + + act(() => { + capturedInterlinearizerProps?.segmentationDispatch.merge('GEN 1:2:0'); + capturedInterlinearizerProps?.segmentationDispatch.split('GEN 1:1:2'); + capturedInterlinearizerProps?.segmentationDispatch.move('GEN 1:1:2', 'GEN 1:1:4'); + }); + + expect(mockSendCommand).not.toHaveBeenCalled(); + }); + + it('drops a phrase mode entered on the draft when an import opens', async () => { + mockImportCommands(); + await act(async () => { + renderLoader(); + }); + act(() => { + capturedInterlinearizerProps?.setPhraseMode({ + kind: 'edit', + phraseId: 'phrase-1', + originalTokens: [{ tokenRef: 'GEN 1:1:0', surfaceText: 'In' }], + }); + }); + expect(capturedInterlinearizerProps?.phraseMode).toEqual({ + kind: 'edit', + phraseId: 'phrase-1', + originalTokens: [{ tokenRef: 'GEN 1:1:0', surfaceText: 'In' }], + }); + + await userEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + await userEvent.click(screen.getByTestId('select-modal-open-import')); + + expect(await screen.findByTestId('pt9-import-banner')).toBeInTheDocument(); + await waitFor(() => + expect(capturedInterlinearizerProps?.phraseMode).toEqual({ kind: 'view' }), + ); + }); + + it('keeps the import view in view mode however the phrase mode is set under it', async () => { + mockImportCommands(); + await renderImportView(); + + act(() => { + capturedInterlinearizerProps?.setPhraseMode({ + kind: 'edit', + phraseId: 'phrase-1', + originalTokens: [{ tokenRef: 'GEN 1:1:0', surfaceText: 'In' }], + }); + }); + + // The reset effect only covers crossing into the import; a mode set from inside it would + // otherwise stand, since nothing crosses back. + expect(capturedInterlinearizerProps?.phraseMode).toEqual({ kind: 'view' }); + }); + it('falls back to the platform language when the import declares no analysis language', async () => { mockImportCommands(); await act(async () => @@ -1559,8 +1745,8 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('interlinearizer')).toBeInTheDocument(); }); - it('notifies when the imported analysis fetch rejects', async () => { - jest.mocked(papi.notifications.send).mockRejectedValue(new Error('ui offline')); + it('reports a rejected imported-analysis fetch in the panel and sends no toast', async () => { + jest.mocked(papi.notifications.send).mockResolvedValue('notification-id'); mockSendCommand.mockImplementation(async (...args) => { if (args[0] === 'interlinearizer.getProject') throw new Error('storage offline'); return JSON.stringify(emptyDraft(testProjectId)); @@ -1569,10 +1755,10 @@ describe('InterlinearizerLoader', () => { renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }), ); - expect(jest.mocked(papi.notifications.send)).toHaveBeenCalledWith({ - message: '%interlinearizer_error_load_projects_failed%', - severity: 'error', - }); + expect(screen.getByTestId('pt9-import-load-error')).toHaveTextContent( + '%interlinearizer_error_pt9Import_load_failed%', + ); + expect(jest.mocked(papi.notifications.send)).not.toHaveBeenCalled(); }); it('shows the in-modal failure when the import result carries no valid report', async () => { @@ -1612,6 +1798,90 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'importPt9'); }); + it('drops an Open fetch that lands after the user has closed the report', async () => { + jest.mocked(papi.notifications.send).mockResolvedValue('notification-id'); + let imported = false; + let releaseSummary: (() => void) | undefined; + mockSendCommand.mockImplementation(async (...args) => { + if (args[0] === 'interlinearizer.getProject' && imported) { + // Hold the summary until the test has taken the user off the report. + await new Promise((resolve) => { + releaseSummary = resolve; + }); + return JSON.stringify(FRESH_IMPORT_SUMMARY); + } + if (args[0] === 'interlinearizer.importPt9Project') { + imported = true; + return JSON.stringify({ + outcome: 'imported', + projectId: 'import-1', + report: IMPORT_REPORT, + }); + } + return JSON.stringify(emptyDraft(testProjectId)); + }); + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + await userEvent.click(screen.getByTestId('select-modal-import-pt9')); + await screen.findByTestId('pt9-import-report'); + + await userEvent.click( + screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_open%' }), + ); + await userEvent.click( + screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }), + ); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'select'); + + await act(async () => { + releaseSummary?.(); + }); + + // The select modal the user went back to is still theirs, and no project has been switched in. + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'select'); + expect(screen.getByTestId('project-modals')).not.toHaveAttribute('data-active-project-name'); + }); + + it('notifies and stays on the report when the Open fetch rejects', async () => { + jest.mocked(papi.notifications.send).mockResolvedValue('notification-id'); + let imported = false; + mockSendCommand.mockImplementation(async (...args) => { + if (args[0] === 'interlinearizer.getProject' && imported) + throw new Error('storage offline'); + if (args[0] === 'interlinearizer.importPt9Project') { + imported = true; + return JSON.stringify({ + outcome: 'imported', + projectId: 'import-1', + report: IMPORT_REPORT, + }); + } + return JSON.stringify(emptyDraft(testProjectId)); + }); + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + await userEvent.click(screen.getByTestId('select-modal-import-pt9')); + await screen.findByTestId('pt9-import-report'); + + await userEvent.click( + screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_open%' }), + ); + + expect(jest.mocked(logger.error)).toHaveBeenCalledWith( + 'Interlinearizer: failed to load the imported project for opening', + expect.any(Error), + ); + expect(jest.mocked(papi.notifications.send)).toHaveBeenCalledWith({ + message: '%interlinearizer_error_load_projects_failed%', + severity: 'error', + }); + expect(screen.getByTestId('pt9-import-report')).toBeInTheDocument(); + }); + it('notifies when the copy command returns no project', async () => { mockImportCommands({ copyJson: '{}' }); jest.mocked(papi.notifications.send).mockRejectedValue(new Error('ui offline')); @@ -1697,8 +1967,8 @@ describe('InterlinearizerLoader', () => { ); }); - it('notifies when the imported analysis fails to load', async () => { - jest.mocked(papi.notifications.send).mockRejectedValue(new Error('ui offline')); + it('reports an imported analysis that never loads in the panel and sends no toast', async () => { + jest.mocked(papi.notifications.send).mockResolvedValue('notification-id'); // An empty response is the never-written case; the effect treats it like a malformed one. mockSendCommand.mockImplementation(async (...args) => args[0] === 'interlinearizer.getProject' ? '' : JSON.stringify(emptyDraft(testProjectId)), @@ -1707,10 +1977,10 @@ describe('InterlinearizerLoader', () => { renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }), ); - expect(jest.mocked(papi.notifications.send)).toHaveBeenCalledWith({ - message: '%interlinearizer_error_load_projects_failed%', - severity: 'error', - }); + expect(screen.getByTestId('pt9-import-load-error')).toHaveTextContent( + '%interlinearizer_error_pt9Import_load_failed%', + ); + expect(jest.mocked(papi.notifications.send)).not.toHaveBeenCalled(); }); }); diff --git a/src/__tests__/components/MorphemeBox.test.tsx b/src/__tests__/components/MorphemeBox.test.tsx index 535cc9cf..bc148919 100644 --- a/src/__tests__/components/MorphemeBox.test.tsx +++ b/src/__tests__/components/MorphemeBox.test.tsx @@ -7,6 +7,7 @@ import type { MorphemeAnalysis } from 'interlinearizer'; import * as AnalysisStore from '../../components/AnalysisStore'; import { MorphemeBox, MorphemeGlossInput } from '../../components/MorphemeBox'; import { TOKEN_CHIP_LABEL_KEYS } from '../../components/PhraseStripContext'; +import { setMockAnalysisReadOnly } from '../analysis-store-read-only-mock'; import { makeWordToken } from '../test-helpers'; jest.mock('../../components/AnalysisStore'); @@ -324,22 +325,8 @@ describe('MorphemeGlossInput', () => { }); }); -/** The manual AnalysisStore mock's test-only controls. */ -interface AnalysisStoreReadOnlyMock { - __setMockAnalysisReadOnly: (value: boolean) => void; -} - -function isAnalysisStoreReadOnlyMock(m: unknown): m is AnalysisStoreReadOnlyMock { - return !!m && typeof m === 'object' && '__setMockAnalysisReadOnly' in m; -} - -const analysisStoreMock: unknown = jest.requireMock('../../components/AnalysisStore'); -if (!isAnalysisStoreReadOnlyMock(analysisStoreMock)) - throw new Error('Expected the AnalysisStore manual mock with read-only controls'); -const { __setMockAnalysisReadOnly: setMockAnalysisReadOnly } = analysisStoreMock; - describe('MorphemeBox read-only', () => { - afterEach(() => { + beforeEach(() => { setMockAnalysisReadOnly(false); }); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 0532b4e7..f3f536c6 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -47,9 +47,12 @@ const mockUsePhraseDispatch = jest.fn, []>().m /** Stable mock fn capturing `useSegmentFreeTranslationDispatch` calls so tests can assert on them. */ const mockSegmentFreeTranslationDispatch = jest.fn(); +/** What the mocked `useAnalysisReadOnly` reports; reset in `beforeEach`. */ +let mockReadOnly = false; + jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, - useAnalysisReadOnly: () => false, + useAnalysisReadOnly: () => mockReadOnly, AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode; analysisLanguage: string }>) { return children; }, @@ -226,6 +229,7 @@ function requiredProps(): { describe('SegmentView', () => { beforeEach(() => { + mockReadOnly = false; mockKeyAsValueLocalizedStrings(); mockUsePhraseLinkMap.mockReturnValue(new Map()); mockUsePhraseDispatch.mockReturnValue({ @@ -585,6 +589,12 @@ describe('SegmentView', () => { expect(screen.queryByTestId('baseline-split-gap')).not.toBeInTheDocument(); }); + it('shows no split gap for a read-only analysis even with Alt held', () => { + mockReadOnly = true; + renderBaseline(); + expect(screen.queryByTestId('baseline-split-gap')).not.toBeInTheDocument(); + }); + it('shows no split gap while a phrase mode is active even with Alt held', () => { renderBaseline({ phraseMode: { kind: 'confirm-unlink', phraseId: 'p1' } }); expect(screen.queryByTestId('baseline-split-gap')).not.toBeInTheDocument(); diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 8d5e6e30..92d2e15a 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -9,6 +9,7 @@ import * as AnalysisStore from '../../components/AnalysisStore'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { InertTokenChip, TokenChip } from '../../components/TokenChip'; import { emptyAnalysis } from '../../types/empty-factories'; +import { setMockAnalysisReadOnly } from '../analysis-store-read-only-mock'; import { FIXTURE_STAMPS, makePunctToken, makeWordToken } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; @@ -788,22 +789,8 @@ describe('TokenChip', () => { }); }); -/** The manual AnalysisStore mock's test-only controls. */ -interface AnalysisStoreReadOnlyMock { - __setMockAnalysisReadOnly: (value: boolean) => void; -} - -function isAnalysisStoreReadOnlyMock(m: unknown): m is AnalysisStoreReadOnlyMock { - return !!m && typeof m === 'object' && '__setMockAnalysisReadOnly' in m; -} - -const analysisStoreMock: unknown = jest.requireMock('../../components/AnalysisStore'); -if (!isAnalysisStoreReadOnlyMock(analysisStoreMock)) - throw new Error('Expected the AnalysisStore manual mock with read-only controls'); -const { __setMockAnalysisReadOnly: setMockAnalysisReadOnly } = analysisStoreMock; - describe('TokenChip read-only', () => { - afterEach(() => { + beforeEach(() => { setMockAnalysisReadOnly(false); }); diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index 7916cf92..7c4e24d7 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -1,7 +1,7 @@ /// /// -import { render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import papi from '@papi/frontend'; import type { DraftProject } from 'interlinearizer'; @@ -31,6 +31,13 @@ const MOCK_PROJECT_2: InterlinearProjectSummary = { name: 'French Project', }; +/** A Paratext 9 import row, which the select modal routes to `onOpenImport`, not the draft. */ +const MOCK_IMPORT_PROJECT: InterlinearProjectSummary = { + ...MOCK_PROJECT, + id: 'import-1', + pt9Import: { fileHashes: {}, importedAt: '2026-08-01T00:00:00.000Z' }, +}; + /** A full project (with analysis) returned by the mocked `interlinearizer.getProject` command. */ const MOCK_FULL_PROJECT = { ...MOCK_PROJECT, analysis: emptyAnalysis() }; @@ -90,6 +97,13 @@ jest.mock('../../../components/modals/SelectInterlinearProjectModal', () => ({ + @@ -1306,6 +1320,29 @@ describe('ProjectModals Paratext 9 import routing', () => { expect(screen.queryByTestId('discard-modal')).not.toBeInTheDocument(); }); + it('holds the picker inert while an import is opening', async () => { + let finishOpen: (() => void) | undefined; + const onOpenImport = jest.fn( + () => + new Promise((resolve) => { + finishOpen = resolve; + }), + ); + render(); + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'false'); + + await userEvent.click(screen.getByTestId('select-select-import')); + + expect(onOpenImport).toHaveBeenCalledWith(MOCK_IMPORT_PROJECT); + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'true'); + + await act(async () => { + finishOpen?.(); + }); + + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'false'); + }); + it('opens an openRequest project through the draft-open flow', async () => { jest .mocked(papi.commands.sendCommand) diff --git a/src/__tests__/utils/pt9-manifest.test.ts b/src/__tests__/utils/pt9-manifest.test.ts new file mode 100644 index 00000000..7c7851d6 --- /dev/null +++ b/src/__tests__/utils/pt9-manifest.test.ts @@ -0,0 +1,51 @@ +/// + +import papi from '@papi/frontend'; +import { PT9_MANIFEST_TIMEOUT_MS, readPt9Manifest } from '../../utils/pt9-manifest'; +import { getMockedPdpGet } from '../test-helpers'; + +const mockPdpGet = getMockedPdpGet(papi); + +describe('readPt9Manifest', () => { + it('resolves the manifest the source project serves', async () => { + mockPdpGet.mockResolvedValue({ + getPt9InterlinearManifest: async () => ({ 'Lexicon.xml': 'aaaa1111' }), + }); + + await expect(readPt9Manifest('src-project')).resolves.toEqual({ 'Lexicon.xml': 'aaaa1111' }); + expect(mockPdpGet).toHaveBeenCalledWith('platformScripture.Pt9Interlinear', 'src-project'); + }); + + it('rejects when the source serves no Pt9Interlinear projectInterface', async () => { + mockPdpGet.mockRejectedValue(new Error('no such projectInterface')); + + await expect(readPt9Manifest('src-project')).rejects.toThrow('no such projectInterface'); + }); + + it('rejects when the read goes unanswered, so a caller behind blocking UI can finish', async () => { + jest.useFakeTimers(); + // A provider that accepts the call and never responds - the hang the timeout exists for. + mockPdpGet.mockResolvedValue({ getPt9InterlinearManifest: () => new Promise(() => {}) }); + + const read = readPt9Manifest('src-project'); + const settled = jest.fn(); + read.catch(settled); + await Promise.resolve(); + expect(settled).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(PT9_MANIFEST_TIMEOUT_MS); + + await expect(read).rejects.toThrow('went unanswered'); + jest.useRealTimers(); + }); + + it('leaves no timer pending once the read answers', async () => { + jest.useFakeTimers(); + mockPdpGet.mockResolvedValue({ getPt9InterlinearManifest: async () => ({}) }); + + await expect(readPt9Manifest('src-project')).resolves.toEqual({}); + + expect(jest.getTimerCount()).toBe(0); + jest.useRealTimers(); + }); +}); diff --git a/src/components/ArcOverlay.tsx b/src/components/ArcOverlay.tsx index bb0efc3a..6793f346 100644 --- a/src/components/ArcOverlay.tsx +++ b/src/components/ArcOverlay.tsx @@ -1,10 +1,11 @@ import type { PhraseAnalysisLink } from 'interlinearizer'; import { Link2Off } from 'lucide-react'; import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'platform-bible-react'; -import { memo, useState, useCallback } from 'react'; +import { memo, useState, useCallback, useEffect } from 'react'; import type { PhraseMode } from '../types/phrase-mode'; import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { computeSplitFreeRefs, getArcStrokeProps, type ArcPath } from '../utils/phrase-arc'; +import { useAnalysisReadOnly } from './AnalysisStore'; /** * Identifies one specific arc boundary by phrase id and the token immediately before the split, @@ -109,9 +110,9 @@ type ArcOverlayProps = Readonly<{ }>; /** - * Renders the phrase-arc SVG layer and (in view mode) the split-button overlay on top of a token - * row. Intended to sit as a sibling of the row inside the `arc-container` element that owns the - * coordinate space the arc paths were measured in. + * Renders the phrase-arc SVG layer and (in view mode, for an editable analysis) the split-button + * overlay on top of a token row. Intended to sit as a sibling of the row inside the `arc-container` + * element that owns the coordinate space the arc paths were measured in. * * @returns The SVG + split-button overlay, or `undefined` when there are no arcs to draw. */ @@ -131,6 +132,9 @@ export function ArcOverlay({ }: ArcOverlayProps) { const [splitHoveredArc, setSplitHoveredArc] = useState(); + // The arcs themselves are the read-only view's phrase rendering; only splitting them is an edit. + const readOnly = useAnalysisReadOnly(); + const splitTooltip = tooltipContentOrUndefined(resolvedOrEmpty(splitHereLabel)); /** @@ -170,6 +174,15 @@ export function ArcOverlay({ onHoverPhrase(undefined); }, [onHoverPhrase]); + // A split button that goes away because the analysis turned read-only never fires its own + // mouse-leave, so whatever preview the hover put up - freed tokens dimmed, or the whole phrase + // highlighted - would stay on a view that no longer offers the split. Take it down here. + useEffect(() => { + if (!readOnly || splitHoveredArc === undefined) return; + if (splitHoveredArc.kind === 'free') handleSplitHoverLeave(); + else handleReshapeHoverLeave(); + }, [readOnly, splitHoveredArc, handleSplitHoverLeave, handleReshapeHoverLeave]); + if (arcPaths.length === 0) return undefined; /** @@ -289,6 +302,7 @@ export function ArcOverlay({ )} {phraseMode.kind === 'view' && + !readOnly && sortedArcPaths // When simplifyPhrases is on, only the focused phrase keeps its split button; every // other phrase's button is hidden while its arc stays drawn. diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index ffed9dca..9a761481 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -31,7 +31,7 @@ import { isInterlinearProjectSummary, isTextAnalysis, isWordToken } from '../typ import { isPt9ImportReport } from '../converters/pt9'; import { toProjectSummary } from '../types/interlinear-project-summary'; import useSubmitGuard from '../hooks/useSubmitGuard'; -import type { SegmentationDispatch } from './SegmentationStore'; +import { NO_OP_SEGMENTATION_DISPATCH, type SegmentationDispatch } from './SegmentationStore'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; import Interlinearizer from './Interlinearizer'; import { AnalysisStoreProvider } from './AnalysisStore'; @@ -51,6 +51,7 @@ import { firstVerseNumber, segmentContainsVerse } from '../utils/verse-ref'; import { resolvedOrEmpty } from '../utils/localized-strings'; import usePanelResizeKeys from '../hooks/usePanelResizeKeys'; import { isPt9TooLargeError } from '../utils/pt9-import-error'; +import { readPt9Manifest } from '../utils/pt9-manifest'; /** Host-injected callback to update this WebView's definition (used to toggle the tab title). */ type UpdateWebViewDefinition = WebViewProps['updateWebViewDefinition']; @@ -154,6 +155,7 @@ const DEFAULT_CATALOG_LAYOUT: PanelLayout = { [VIEW_PANEL_ID]: 75, [CATALOG_PANE const STRING_KEYS = [ '%interlinearizer_error_load_book_heading%', '%interlinearizer_error_process_book_heading%', + '%interlinearizer_error_pt9Import_load_failed%', '%interlinearizer_loading%', '%interlinearizer_analysisCatalog_resize%', '%interlinearizer_banner_pt9Import%', @@ -167,6 +169,9 @@ const STRING_KEYS = [ */ const PT9_CHECKING_DELAY_MS = 400; +/** The phrase mode a read-only view is always in, shared so its identity stays stable. */ +const VIEW_PHRASE_MODE: PhraseMode = { kind: 'view' }; + /** The provenance an import project carries; the open-import path requires it present. */ type Pt9ImportProvenance = NonNullable; @@ -305,48 +310,52 @@ function InterlinearizerLoaderInner({ const isImportView = activeProject?.pt9Import !== undefined; /** - * The import's stored analysis, fetched fresh whenever the import view opens or a sync bumps the - * project's `updatedAt`. Never the draft: the user's in-progress draft survives viewing an import - * untouched. + * Which version of the import the view is on: its id and the modification time a sync bumps. + * `undefined` while the draft is the view. + */ + const importTag = + isImportView && activeProject ? `${activeProject.id}:${activeProject.updatedAt}` : undefined; + + /** + * The import analysis last fetched, under the version tag it was fetched for; no `analysis` when + * that fetch found none to show. An analysis the sync has already replaced is therefore never one + * the view can paint. */ - const [importAnalysis, setImportAnalysis] = useState(undefined); + const [importLoad, setImportLoad] = useState<{ tag: string; analysis?: TextAnalysis }>(); useEffect(() => { - if (!isImportView || !activeProject) { - setImportAnalysis(undefined); - return undefined; - } + if (importTag === undefined || !activeProject) return undefined; + const { id } = activeProject; let ignore = false; (async () => { try { - const json = await papi.commands.sendCommand( - 'interlinearizer.getProject', - activeProject.id, - ); + const json = await papi.commands.sendCommand('interlinearizer.getProject', id); const parsed: unknown = json ? JSON.parse(json) : undefined; const analysis = parsed && typeof parsed === 'object' && 'analysis' in parsed ? parsed.analysis : undefined; if (ignore) return; - if (isTextAnalysis(analysis)) { - setImportAnalysis(analysis); - } else { - await papi.notifications - .send({ message: '%interlinearizer_error_load_projects_failed%', severity: 'error' }) - .catch(() => {}); - } + // Either outcome is reported by the panel's own line rather than a toast: it stays on + // screen next to the empty view, and there is only one message to reconcile. + setImportLoad(isTextAnalysis(analysis) ? { tag: importTag, analysis } : { tag: importTag }); } catch (e) { logger.error('Interlinearizer: failed to load the imported analysis', e); - await papi.notifications - .send({ message: '%interlinearizer_error_load_projects_failed%', severity: 'error' }) - .catch(() => {}); + if (!ignore) setImportLoad({ tag: importTag }); } })(); return () => { ignore = true; }; - // eslint-disable-next-line react-hooks/exhaustive-deps -- updatedAt stands in for the analysis a sync replaced - }, [isImportView, activeProject?.id, activeProject?.updatedAt]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- the tag names the version to fetch; the project object also changes for edits that leave the analysis alone + }, [importTag]); + + /** The fetch's outcome for the version on screen; `undefined` until that version has one. */ + const importLoaded = importLoad?.tag === importTag ? importLoad : undefined; + + const importAnalysis = importLoaded?.analysis; + + /** Whether the version on screen is one whose analysis could not be read. */ + const importLoadFailed = importLoaded !== undefined && importLoaded.analysis === undefined; // Whether any gloss input currently holds uncommitted text. Gloss writes are deferred to blur, so // the persisted `dirty` flag does not flip until then; tracking in-progress edits here lets the @@ -485,12 +494,17 @@ function InterlinearizerLoaderInner({ }, [verseBook, segmentationVersion, draftVersion, isDraftLoading]); /** - * Boundary-editing operations exposed through the segmentation context. Each reads the draft's - * latest boundary delta synchronously (so rapid edits compose correctly), applies the relevant - * pure transform against the original verse book, and auto-saves the normalized result — clearing - * the field back to `undefined` when the edit restores the default verse segmentation. + * Boundary-editing operations exposed through the segmentation context, inert while an import is + * the view. Each reads the draft's latest boundary delta synchronously (so rapid edits compose + * correctly), applies the relevant pure transform against the original verse book, and auto-saves + * the normalized result — clearing the field back to `undefined` when the edit restores the + * default verse segmentation. */ const segmentationDispatch = useMemo(() => { + // An import is read-only and its view is not backed by the draft, so a boundary edit reached + // from it has nowhere legitimate to land: the controls are absent there, and this keeps any + // that slips through from rewriting the draft the import is preserving. + if (isImportView) return NO_OP_SEGMENTATION_DISPATCH; /** * Auto-saves the result of a boundary transform, clearing the segmentation field back to * `undefined` when the edit restores the default verse segmentation. @@ -515,7 +529,7 @@ function InterlinearizerLoaderInner({ apply(moveBoundary(verseBook, getDraftSnapshot()?.segmentation, fromRef, toRef)); }, }; - }, [verseBook, getDraftSnapshot, autosaveSegmentation]); + }, [autosaveSegmentation, getDraftSnapshot, isImportView, verseBook]); // The active reference handed to the interlinearizer. The host emits `verseNum: 0` both for a // chapter's verse-0 superscription (which has its own segment) and for a plain whole-chapter @@ -586,25 +600,37 @@ function InterlinearizerLoaderInner({ const [modal, setModal] = useState('none'); + /** + * The modal on screen, for an async handler that must not act on a dialog the user has left. + * Assigned during render rather than from an effect so a promise resolving in the same tick as + * the dismissal still sees the move. + */ + const modalRef = useRef(modal); + modalRef.current = modal; + /** Whether the destructive wipe dialog (book / whole-draft scope picker) is open. */ const [wipeModalOpen, setWipeModalOpen] = useState(false); - const [phraseMode, setPhraseMode] = useState({ kind: 'view' }); + const [phraseMode, setPhraseMode] = useState(VIEW_PHRASE_MODE); - // Reset phraseMode whenever the draft is replaced wholesale (New / Open / Wipe) so stale - // edit/confirm-unlink state is never passed to the newly mounted Interlinearizer. + // Reset phraseMode whenever the draft is replaced wholesale (New / Open / Wipe), and whenever the + // view crosses between the draft and an import, so stale edit/confirm-unlink state is never + // passed to the newly mounted Interlinearizer. An import opens without touching the draft or its + // version, and a mode carried into that read-only view renders its edit-target affordances. + // Crossing into the import is all this covers; the render below pins the import view's mode + // outright, since a mode set from inside that view has no crossing to reset it. useEffect(() => { - setPhraseMode({ kind: 'view' }); - }, [draftVersion]); + setPhraseMode(VIEW_PHRASE_MODE); + }, [draftVersion, isImportView]); /** What the Paratext 9 import modal shows while `modal` is `'importPt9'`. */ const [pt9Phase, setPt9Phase] = useState({ kind: 'running' }); /** - * Which run the import modal belongs to: a first import from the select modal or the first-open - * offer (report offers Open; closing returns where the run began), a manual sync (report offers - * Close), or the automatic sync on open (running state only; the view opens itself when the run - * settles). + * Which run the import modal belongs to: a first import from the select modal (report offers + * Close and Open, closing returning to the select modal), the accepted first-open offer (report + * offers Open alone, dismissal included), a manual sync (report offers Close), or the automatic + * sync on open (running state only; the view opens itself when the run settles). */ const [pt9Mode, setPt9Mode] = useState<'import' | 'offer' | 'sync' | 'autoSync'>('import'); @@ -684,17 +710,14 @@ function InterlinearizerLoaderInner({ /** * Opens a Paratext 9 import from the select modal: probes the manifest and, when the source files * changed since the last import, syncs first behind the import modal's running state - closing - * straight into the view, with no report step on the open path. Every failure opens the stored - * (stale) import with one warning instead of blocking access to it. + * straight into the view, with no report step on the open path. Every failure - a manifest read + * that never answers included - opens the stored (stale) import with one warning instead of + * blocking access to it. */ const openImportedProject = useCallback( async (project: InterlinearProjectSummary & { pt9Import: Pt9ImportProvenance }) => { try { - const pdp = await papi.projectDataProviders.get( - 'platformScripture.Pt9Interlinear', - projectId, - ); - const manifest = await pdp.getPt9InterlinearManifest(); + const manifest = await readPt9Manifest(projectId); if (fileHashesEqual(manifest, project.pt9Import.fileHashes)) { setActiveProject(project); setModal('none'); @@ -729,11 +752,22 @@ function InterlinearizerLoaderInner({ [projectId, fetchSummary, setActiveProject], ); - /** Opens the freshly imported project from the report into the read-only view. */ + /** + * Opens the freshly imported project from the report into the read-only view. A fetch that fails + * leaves the report standing, so the Open can be taken again once whatever broke is fixed. The + * report stays dismissable while that fetch is in flight, and one settling after the user has + * left it neither switches the project nor reports into whatever they moved on to. + */ const handlePt9Open = useCallback(async () => { /* v8 ignore next -- Open only renders on a report, which always sets the imported id first */ if (!pt9ImportedId) return; - const summary = await fetchSummary(pt9ImportedId); + let summary: InterlinearProjectSummary | undefined; + try { + summary = await fetchSummary(pt9ImportedId); + } catch (e) { + logger.error('Interlinearizer: failed to load the imported project for opening', e); + } + if (modalRef.current !== 'importPt9') return; if (summary) { setActiveProject(summary); setModal('none'); @@ -1052,6 +1086,12 @@ function InterlinearizerLoaderInner({ {resolvedOrEmpty(localizedStrings['%interlinearizer_loading%'])}

)} + + {!hasError && !showLoading && importLoadFailed && ( +

+ {localizedStrings['%interlinearizer_error_pt9Import_load_failed%']} +

+ )} ); @@ -1064,7 +1104,7 @@ function InterlinearizerLoaderInner({ book={book} continuousScroll={continuousScroll} scrRef={activeScrRef} - phraseMode={phraseMode} + phraseMode={isImportView ? VIEW_PHRASE_MODE : phraseMode} setPhraseMode={setPhraseMode} viewOptions={viewOptions} segmentationDispatch={segmentationDispatch} @@ -1127,10 +1167,10 @@ function InterlinearizerLoaderInner({ importAnalysis === undefined ? ( {loadingOrErrorPanel} ) : ( - // Keyed on id + updatedAt so a sync (which bumps updatedAt) reseeds by remounting, the - // same non-reactive-seed contract the draft-backed store relies on. + // Keyed on the version tag so a sync reseeds by remounting, the same non-reactive-seed + // contract the draft-backed store relies on. diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 435e4a45..cd71a7d3 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -15,6 +15,7 @@ import { buildSegmentLabels } from '../utils/segment-labels'; import { segmentContainsVerse } from '../utils/verse-ref'; import { buildVerseStartLabels } from '../utils/verse-superscripts'; import { useAltHeldValue } from './AltHeldContext'; +import { useAnalysisReadOnly } from './AnalysisStore'; import { useFocus, useFocusActions } from './FocusStore'; import { useSegmentation } from './SegmentationStore'; import MemoizedSegmentView from './SegmentView'; @@ -193,6 +194,7 @@ export default function SegmentListView({ }: SegmentListViewProps) { const { tokenRef: focusedTokenRef } = useFocus(); const { selectSegment } = useFocusActions(); + const readOnly = useAnalysisReadOnly(); const [localizedStrings] = useLocalizedStrings(HEADER_STRING_KEYS); const recenterTooltip = tooltipContentOrUndefined( @@ -420,9 +422,10 @@ export default function SegmentListView({ // window: merge dispatches against the delta, not the DOM, so the topmost windowed // segment's boundary with a culled predecessor is still editable. const canMerge = mergeableSegmentIds.has(seg.id); - // Omit the merge control while a phrase mode is active: a merge could re-segment the - // phrase the mode UI is operating on. - const showMergeControl = canMerge && phraseMode.kind === 'view'; + // Omit the merge control while a phrase mode is active (a merge could re-segment the + // phrase the mode UI is operating on) and for a read-only analysis, which offers no + // boundary editing at all. + const showMergeControl = canMerge && phraseMode.kind === 'view' && !readOnly; return ( {showMergeControl && } diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 0f55d614..95960090 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -19,8 +19,8 @@ import { buildRenderUnits, groupTokens, resolveFocusContext } from '../utils/tok import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { resolveSplitAnchor } from '../utils/split-anchor'; import { slotVerseLabel, verseStartToken } from '../utils/verse-superscripts'; -import { usePhraseLinkByIdMap, usePhraseLinkMap } from './AnalysisStore'; import { useAltHeldValue } from './AltHeldContext'; +import { useAnalysisReadOnly, usePhraseLinkByIdMap, usePhraseLinkMap } from './AnalysisStore'; import MemoizedArcOverlay from './ArcOverlay'; import SegmentFreeTranslationInput from './SegmentFreeTranslationInput'; import { PhraseStripProvider } from './PhraseStripContext'; @@ -302,6 +302,7 @@ export function SegmentView({ const [localizedStrings] = useLocalizedStrings(STRING_KEYS); const { dispatch, formerBoundaries, straddledBoundaryRefs } = useSegmentation(); + const readOnly = useAnalysisReadOnly(); const phraseLinkByRef = usePhraseLinkMap(); const phraseLinkById = usePhraseLinkByIdMap(); @@ -363,19 +364,27 @@ export function SegmentView({ /** * Split anchor by the char offset of the gap that precedes it, for baseline-text mode: for each * eligible word-word pair the split anchor's leading gap (the inter-token region just before the - * anchor token) becomes a splittable gap. A pair is eligible only in `view` mode and only when - * its word boundary is not a mid-phrase (straddled) boundary — the same rules the token-chip - * marker uses. A split at a former boundary dispatches the original removed default start (which - * may be leading punctuation) so the delta can normalize back to the default segmentation; - * otherwise the anchor comes from the punctuation-travel rule. The gap is keyed by the offset of - * the token the split actually lands before (the dispatched ref's own token), so the highlighted - * caret sits exactly where the boundary will fall — including a former boundary whose - * leading-punctuation ref is a few characters left of the word anchor. The `altHeld` gate is - * applied at render time, not here, so this map stays stable across Alt presses. + * anchor token) becomes a splittable gap. + * + * A pair is eligible only in `view` mode, only when the analysis is editable (a read-only one has + * no splittable gap anywhere), and only when its word boundary is not a mid-phrase (straddled) + * boundary — the same rules the token-chip marker uses. + * + * A split at a former boundary dispatches the original removed default start (which may be + * leading punctuation) so the delta can normalize back to the default segmentation; otherwise the + * anchor comes from the punctuation-travel rule. + * + * The gap is keyed by the offset of the token the split actually lands before (the dispatched + * ref's own token), so the highlighted caret sits exactly where the boundary will fall — + * including a former boundary whose leading-punctuation ref is a few characters left of the word + * anchor. + * + * The `altHeld` gate is applied at render time, not here, so this map stays stable across Alt + * presses. */ const splitGapByOffset = useMemo(() => { const map = new Map(); - if (phraseMode.kind !== 'view') return map; + if (readOnly || phraseMode.kind !== 'view') return map; const { tokens, baselineText } = segment; const tokenByRef = new Map(tokens.map((t) => [t.ref, t])); let prevWord: Token | undefined; @@ -398,7 +407,7 @@ export function SegmentView({ pendingPunct = []; }); return map; - }, [segment, phraseMode.kind, straddledBoundaryRefs, formerBoundaries]); + }, [formerBoundaries, phraseMode.kind, readOnly, segment, straddledBoundaryRefs]); /** * The ordered baseline-text render pieces: plain-text runs, inline verse superscripts, and diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index fedbfae6..54174302 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -62,7 +62,8 @@ type PendingReplace = * modal; the caller owns the import run and its modal. * @param props.onOpenImport - Called instead of the draft-open flow when the user picks a Paratext * 9 import in the select modal: the import opens read-only without touching the draft, so no - * unsaved-work confirmation is needed. + * unsaved-work confirmation is needed. Resolves once the import is open, which may be after a + * sync; the select modal stays inert until then. * @param props.openRequest - A project the caller asks to open through the normal draft-open flow, * unsaved-work confirmation included; each new `requestId` performs one open. Used to open a * freshly created editable copy. @@ -104,7 +105,7 @@ export default function ProjectModals({ project: InterlinearProjectSummary & { pt9Import: NonNullable; }, - ) => void; + ) => Promise; openRequest?: { project: InterlinearProjectSummary; requestId: number }; projectId: string; setModal: (modal: ModalState) => void; @@ -154,9 +155,10 @@ export default function ProjectModals({ const replaceGuard = useSubmitGuard(); /** - * Guards opening a project straight from the select modal — the path taken when the draft has no - * unsaved work, so no confirmation intervenes. `isSubmitting` suppresses that modal's dismissal - * while the open is in flight. An open deferred behind the confirmation has its own guard. + * Guards opening a project straight from the select modal — a Paratext 9 import, or an ordinary + * project when the draft has no unsaved work, so no confirmation intervenes. `isSubmitting` + * suppresses that modal's dismissal while the open is in flight. An open deferred behind the + * confirmation has its own guard. */ const openGuard = useSubmitGuard(); @@ -320,9 +322,12 @@ export default function ProjectModals({ // A Paratext 9 import opens read-only without touching the draft, so it needs neither the // unsaved-work confirmation nor the draft-open flow. Rebuilt with the provenance the check // just proved present, since narrowing one property does not narrow the object being passed. + // Guarded like the draft open beside it: opening an import can take a manifest probe and a + // whole sync, and an unguarded modal would stay live for the duration - long enough for a + // second choice to settle first and then be overwritten by the import arriving late. const { pt9Import } = project; if (pt9Import !== undefined) { - onOpenImport({ ...project, pt9Import }); + openGuard.runGuarded(() => onOpenImport({ ...project, pt9Import })); return; } if (hasUnsavedWork) setPendingReplace({ kind: 'open', project }); diff --git a/src/components/modals/Pt9ImportModal.tsx b/src/components/modals/Pt9ImportModal.tsx index d1998e5b..6608b207 100644 --- a/src/components/modals/Pt9ImportModal.tsx +++ b/src/components/modals/Pt9ImportModal.tsx @@ -118,8 +118,8 @@ function ReportRow({ label, value }: Readonly<{ label: string; value: string }>) * report; `offer` titles it the same but the report carries a single Open, dismissal included - * the user already chose to convert, so the report is information on the way in, not a fork; * `sync` titles it as a sync and offers only Close (the project is already open). - * @param props.onOpen - Called when the user opens the imported project from the report; only - * rendered in `import` mode. + * @param props.onOpen - Called when the user opens the imported project from the report; not + * offered in `sync` mode, where the project is already open. * @param props.onClose - Called when the user dismisses the report or failure. */ export function Pt9ImportModal({ diff --git a/src/components/modals/SelectInterlinearProjectModal.tsx b/src/components/modals/SelectInterlinearProjectModal.tsx index a47c602c..5d30975e 100644 --- a/src/components/modals/SelectInterlinearProjectModal.tsx +++ b/src/components/modals/SelectInterlinearProjectModal.tsx @@ -29,11 +29,11 @@ const SELECT_INTERLINEAR_PROJECT_STRING_KEYS: `%${string}%`[] = [ * @param props.activeProjectId - ID of the project currently open as the active Save target, if * any; the matching list entry is highlighted and badged so the user can tell which project the * draft is currently working against. - * @param props.isOpening - When `true`, a project the user already chose is still being loaded into - * the draft, so the modal's controls go inert for the duration: the open completes regardless, so - * letting the modal be dismissed would read as having canceled it, and choosing another project - * would race the open already in flight. A caller may leave it `false` for an open the user - * cannot reach this modal behind. + * @param props.isOpening - When `true`, a project the user already chose is still opening, so the + * modal's controls go inert for the duration: the open completes regardless, so letting the modal + * be dismissed would read as having canceled it, and choosing another project would race the open + * already in flight. A caller may leave it `false` for an open the user cannot reach this modal + * behind. * @param props.onSelect - Called with the chosen project when the user picks an existing one. * @param props.onCreateNew - Called when the user chooses to create a new project instead. * @param props.onImportPt9 - Called when the user chooses to import the source's Paratext 9 diff --git a/src/hooks/usePt9ImportAvailability.ts b/src/hooks/usePt9ImportAvailability.ts index 6341d160..47174cbb 100644 --- a/src/hooks/usePt9ImportAvailability.ts +++ b/src/hooks/usePt9ImportAvailability.ts @@ -1,6 +1,7 @@ -import papi, { logger } from '@papi/frontend'; +import { logger } from '@papi/frontend'; import { useEffect, useState } from 'react'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; +import { readPt9Manifest } from '../utils/pt9-manifest'; /** What a probe knows so far: undetermined, found data, or found none (failures included). */ export type Pt9ProbeState = 'pending' | 'available' | 'unavailable'; @@ -8,9 +9,9 @@ export type Pt9ProbeState = 'pending' | 'available' | 'unavailable'; /** * Probes whether the source project serves Paratext 9 interlinear data, one manifest read per * enablement: `pending` until an enabled probe answers, `available` when the manifest lists any - * file, `unavailable` when it is empty or the probe fails - a source without the projectInterface - * is the common failure, and it simply has nothing to import. Each probe starts from `pending`, so - * a failing re-probe cannot leave a stale answer standing. + * file, `unavailable` when it is empty or the read fails - a source without the projectInterface is + * the common failure, and it simply has nothing to import. Each probe starts from `pending`, so a + * failing re-probe cannot leave a stale answer standing. * * @param sourceProjectId - Platform.Bible project ID to probe. * @param enabled - Whether to probe at all; while `false` the state stays `pending`. @@ -24,11 +25,7 @@ export function usePt9ImportProbe(sourceProjectId: string, enabled: boolean): Pt let ignore = false; (async () => { try { - const pdp = await papi.projectDataProviders.get( - 'platformScripture.Pt9Interlinear', - sourceProjectId, - ); - const manifest = await pdp.getPt9InterlinearManifest(); + const manifest = await readPt9Manifest(sourceProjectId); if (!ignore) setState(Object.keys(manifest).length > 0 ? 'available' : 'unavailable'); } catch (e) { logger.debug(`Interlinearizer: PT9 import probe failed for ${sourceProjectId}`, e); diff --git a/src/utils/pt9-manifest.ts b/src/utils/pt9-manifest.ts new file mode 100644 index 00000000..73af0865 --- /dev/null +++ b/src/utils/pt9-manifest.ts @@ -0,0 +1,41 @@ +import papi from '@papi/frontend'; + +/** + * How long a manifest read may go unanswered before {@link readPt9Manifest} gives up on it. A + * project data provider that accepts the call and never responds would otherwise leave its caller + * waiting for the life of the tab. + */ +export const PT9_MANIFEST_TIMEOUT_MS = 15_000; + +/** + * Reads the source project's Paratext 9 interlinear manifest: every interlinear file it serves, by + * path, with the hash an import compares against to tell whether the source has changed. An empty + * manifest means the source serves no interlinear data at all. + * + * @throws If the source has no `platformScripture.Pt9Interlinear` projectInterface, the read + * rejects, or it goes unanswered for {@link PT9_MANIFEST_TIMEOUT_MS}. That timeout bounds the + * wait, not the read: PAPI offers no cancellation, so an unanswered read stays outstanding and + * its late result is dropped. + */ +export function readPt9Manifest(sourceProjectId: string): Promise> { + let timer: ReturnType | undefined; + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout( + () => + reject( + new Error( + `Paratext 9 interlinear manifest read for ${sourceProjectId} went unanswered after ${PT9_MANIFEST_TIMEOUT_MS}ms`, + ), + ), + PT9_MANIFEST_TIMEOUT_MS, + ); + }); + const read = (async () => { + const pdp = await papi.projectDataProviders.get( + 'platformScripture.Pt9Interlinear', + sourceProjectId, + ); + return pdp.getPt9InterlinearManifest(); + })(); + return Promise.race([read, timeout]).finally(() => clearTimeout(timer)); +} diff --git a/user-questions.md b/user-questions.md index 73c95f43..e6c8b434 100644 --- a/user-questions.md +++ b/user-questions.md @@ -312,8 +312,9 @@ Decisions made during development that we'd like reviewed: match the project's text"), the view header banner ("Imported from Paratext 9 - read-only - last synced {date}"), the copy dialog's prefilled name ("Copy of Paratext 9 Interlinear"), and the warning shown when a sync cannot refresh ("...couldn't be refreshed; showing the last - imported data"). Do these read right, and should "Paratext 9" be spelled out or abbreviated - anywhere? + imported data"), and what the view says in place of an import whose data cannot be read ("The + imported interlinear data could not be loaded. Try syncing from Paratext 9."). Do these read + right, and should "Paratext 9" be spelled out or abbreviated anywhere? ## First-open offer to convert Paratext 9 interlinear data