From f3664d7d0f23a47088537910ee6bcbdf1b47d21b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 14 Oct 2026 16:09:40 -0600 Subject: [PATCH 01/10] Check analyses against the whole source text (#143) --- __mocks__/lucide-react.tsx | 7 + contributions/localizedStrings.json | 3 + docs/reanchoring.md | 2 +- .../components/AnalysisCatalogPanel.test.tsx | 153 +++++++++++- .../components/CatalogMergeModal.test.tsx | 1 + .../components/CatalogRowHeader.test.tsx | 16 ++ .../ConcordanceIndexContext.test.tsx | 2 + .../components/ConcordancePanel.test.tsx | 11 +- .../components/InterlinearizerLoader.test.tsx | 217 +++++++++++++++++- .../components/TextReadingStatus.test.tsx | 38 +++ .../hooks/useConcordanceIndex.test.ts | 116 +++++++++- src/__tests__/store/analysisSlice.test.ts | 1 + src/__tests__/utils/analysis-query.test.ts | 34 +++ src/__tests__/utils/merge-content.test.ts | 1 + src/__tests__/utils/reanchor-draft.test.ts | 42 +++- .../utils/stale-free-translations.test.ts | 50 +++- src/components/AnalysisCatalogPanel.tsx | 126 ++++++---- src/components/CatalogFilterPopover.tsx | 26 ++- src/components/CatalogList.tsx | 2 +- src/components/CatalogQueryControls.tsx | 4 + src/components/CatalogRowHeader.tsx | 19 +- src/components/CatalogRowView.tsx | 4 + src/components/ConcordancePanel.tsx | 27 +-- src/components/InterlinearizerLoader.tsx | 60 ++++- src/components/TextReadingStatus.tsx | 48 ++++ .../__mocks__/TextReadingStatus.tsx | 14 ++ src/hooks/useConcordanceIndex.ts | 78 +++++-- src/utils/analysis-book.ts | 9 + src/utils/analysis-query.ts | 21 +- src/utils/reanchor-draft.ts | 11 + src/utils/stale-free-translations.ts | 49 +++- 31 files changed, 1072 insertions(+), 120 deletions(-) create mode 100644 src/__tests__/components/TextReadingStatus.test.tsx create mode 100644 src/components/TextReadingStatus.tsx create mode 100644 src/components/__mocks__/TextReadingStatus.tsx diff --git a/__mocks__/lucide-react.tsx b/__mocks__/lucide-react.tsx index 06b34b74a..3daeb3290 100644 --- a/__mocks__/lucide-react.tsx +++ b/__mocks__/lucide-react.tsx @@ -26,6 +26,13 @@ export function GripVertical(props: Readonly<{ className?: string }>): ReactElem return ; } +/** + * Stub for the SearchX icon. + */ +export function SearchX(props: Readonly<{ className?: string }>): ReactElement { + return ; +} + /** * Stub for the Info icon. */ diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index e5e8b2183..8ee5cd464 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -82,6 +82,8 @@ "%interlinearizer_analysisCatalog_filter_morphemes_lacks%": "Not broken down", "%interlinearizer_analysisCatalog_filter_zeroUsages%": "Used nowhere", "%interlinearizer_analysisCatalog_filter_stale%": "Stale (found when a book is opened)", + "%interlinearizer_analysisCatalog_filter_staleEverywhere%": "Stale", + "%interlinearizer_analysisCatalog_filter_notInText%": "Word not in the text", "%interlinearizer_analysisCatalog_confidence_high%": "High", "%interlinearizer_analysisCatalog_confidence_medium%": "Medium", "%interlinearizer_analysisCatalog_confidence_low%": "Low", @@ -94,6 +96,7 @@ "%interlinearizer_analysisCatalog_noUsages%": "Not used anywhere", "%interlinearizer_analysisCatalog_showAllUsages%": "Show {count} more", "%interlinearizer_analysisCatalog_staleCount%": "Stale places", + "%interlinearizer_analysisCatalog_notInText%": "Not in the text", "%interlinearizer_analysisCatalog_staleHeading%": "Stale: the text changed here after approval", "%interlinearizer_analysisCatalog_staleApply%": "Apply to “{word}”", "%interlinearizer_analysisCatalog_staleApplyNoTarget%": "Select a word to apply to", diff --git a/docs/reanchoring.md b/docs/reanchoring.md index c06efd804..1f383136d 100644 --- a/docs/reanchoring.md +++ b/docs/reanchoring.md @@ -21,7 +21,7 @@ Do not grow this into a general diff, fuzzy matching, or cross-verse tracking. F ## Algorithm -When it runs: on every book load, in `useReanchorToBook` (`src/components/AnalysisStore.tsx`), for an editable project only. An imported, read-only project is a record of what was imported and is never healed. A pass that moves nothing leaves the analysis identical, so opening a book neither dirties the draft nor writes storage. +When it runs: whenever the loaded book's text, its boundaries, or the draft changes, and for every other book the draft has records in each time the whole source text is read (for the concordance, or a catalog filter that needs the text), for an editable project only. A book the project no longer has is treated as holding no text, so every approval in it goes stale. An imported, read-only project is a record of what was imported and is never healed. A pass that moves nothing leaves the analysis identical, so opening a book neither dirties the draft nor writes storage. Token and phrase links carry a `TokenSnapshot` (`tokenRef` plus the `surfaceText` it was written against). Approved, candidate, and stale links are the evidence the alignment weighs; rejected and suggested links align afterward, among themselves, against the tokens those leave unclaimed, so a verdict never decides which occurrence an approval names. For each verse independently: diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 421171515..4a74b7f4c 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -11,13 +11,18 @@ import AnalysisCatalogPanel, { type AnalysisCatalogPanelHandle, } from '../../components/AnalysisCatalogPanel'; import { AnalysisStoreProvider, useGlossDispatch } from '../../components/AnalysisStore'; +import { ConcordanceIndexContext } from '../../components/ConcordanceIndexContext'; import { InterlinearNavProvider, useInterlinearNav } from '../../components/InterlinearNavContext'; +import type { ConcordanceIndex } from '../../hooks/useConcordanceIndex'; import type { StepSummary } from '../../hooks/useDraftProject'; import { emptyAnalysis } from '../../types/empty-factories'; import type { HeadingPlacement } from '../../utils/analysis-query'; import { defaultScrRef, FIXTURE_STAMPS, makeScrollGroupHook } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; +// How reading progress and failure read is the status's own concern. +jest.mock('../../components/TextReadingStatus'); + const NO_HEADINGS: ReadonlyMap = new Map(); /** @@ -83,6 +88,20 @@ function FocusPublishProbe({ tokenRef }: Readonly<{ tokenRef: string | undefined return undefined; } +/** Builds an index of the source text as its hook hands it over, unread by default. */ +function makeIndex(overrides: Partial = {}): ConcordanceIndex { + return { + status: 'idle', + booksRead: 0, + bookCount: 0, + entries: [], + textForms: undefined, + refresh: () => {}, + request: () => {}, + ...overrides, + }; +} + /** Options every `renderPanel` call may override. */ type PanelOptions = Partial<{ ref: Ref; @@ -111,6 +130,10 @@ type PanelOptions = Partial<{ liveSurfaceText: (tokenRef: string) => string | undefined; /** The word the view beside the panel has focused, or none. */ focusedTokenRef: string; + /** The index of the source text. Defaults to one not yet read. */ + index: ConcordanceIndex; + /** Whether every book has been checked for stale places. Defaults to only the opened ones. */ + staleCoversDraft: boolean; }>; /** Reads every token as still carrying the form its analysis was recorded under. */ @@ -143,7 +166,9 @@ function PanelProviders({ > - {children} + + {children} + ); @@ -164,6 +189,7 @@ function renderPanel(overrides: PanelOptions = {}) { onShowConcordance={overrides.onShowConcordance ?? (() => {})} showMorphology={overrides.showMorphology ?? true} sourceLanguageTag="el" + staleCoversDraft={overrides.staleCoversDraft ?? false} /> , ); @@ -189,6 +215,7 @@ function ReopenableCatalog() { onShowConcordance={() => {}} showMorphology sourceLanguageTag="el" + staleCoversDraft={false} /> )} @@ -229,6 +256,7 @@ function renderPanelWithGlossEditing(overrides: PanelOptions = {}) { onShowConcordance={overrides.onShowConcordance ?? (() => {})} showMorphology={overrides.showMorphology ?? true} sourceLanguageTag="el" + staleCoversDraft={false} /> , ); @@ -1222,6 +1250,7 @@ describe('AnalysisCatalogPanel', () => { onShowConcordance={() => {}} showMorphology={false} sourceLanguageTag="el" + staleCoversDraft={false} /> , ); @@ -1379,6 +1408,7 @@ describe('AnalysisCatalogPanel', () => { onShowConcordance={() => {}} showMorphology sourceLanguageTag="el" + staleCoversDraft={false} /> , ); @@ -1403,6 +1433,7 @@ describe('AnalysisCatalogPanel', () => { onShowConcordance={() => {}} showMorphology sourceLanguageTag="el" + staleCoversDraft={false} /> , ); @@ -1998,6 +2029,38 @@ describe('AnalysisCatalogPanel', () => { }); }); + it('asks for the text to be read when filtering for them', async () => { + const request = jest.fn(); + renderPanel({ analysis: ONLY_STALE, index: makeIndex({ request }) }); + await openFilters(); + + await userEvent.click( + screen.getByRole('checkbox', { name: '%interlinearizer_analysisCatalog_filter_stale%' }), + ); + + expect(request).toHaveBeenCalled(); + }); + + it('says the stale filter finds them only in opened books until every book is checked', async () => { + renderPanel({ analysis: ONLY_STALE }); + await openFilters(); + + expect( + screen.getByRole('checkbox', { name: '%interlinearizer_analysisCatalog_filter_stale%' }), + ).toBeInTheDocument(); + }); + + it('names the stale filter plainly once every book is checked', async () => { + renderPanel({ analysis: ONLY_STALE, staleCoversDraft: true }); + await openFilters(); + + expect( + screen.getByRole('checkbox', { + name: '%interlinearizer_analysisCatalog_filter_staleEverywhere%', + }), + ).toBeInTheDocument(); + }); + describe('over an unsaved breakdown', () => { /** Types a re-segmentation into an expanded row's breakdown without saving it. */ async function typeUnsavedBreakdown(analysisId = 'ta-1'): Promise { @@ -2124,6 +2187,94 @@ describe('AnalysisCatalogPanel', () => { expect(onClose).toHaveBeenCalled(); }); + describe('forms not in the text', () => { + /** One analysis whose form the text still holds, and one whose form it no longer does. */ + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'kept', surfaceText: 'λόγος' }, + { ...FIXTURE_STAMPS, id: 'gone', surfaceText: 'ἦν' }, + ], + tokenAnalysisLinks: [ + link('kept', 'GEN 1:1:0', 'approved', 'λόγος'), + link('gone', 'GEN 1:2:0', 'stale', 'ἦν'), + ], + }; + + /** The filter keeping only analyses whose form the text no longer holds. */ + function notInTextFilter(): HTMLElement { + return screen.getByRole('checkbox', { + name: '%interlinearizer_analysisCatalog_filter_notInText%', + }); + } + + it('asks for the text to be read when filtering for them', async () => { + const request = jest.fn(); + renderPanel({ analysis, index: makeIndex({ request }) }); + await openFilters(); + + await userEvent.click(notInTextFilter()); + + expect(request).toHaveBeenCalled(); + }); + + it('keeps only the analyses whose form the text lacks once it is read', async () => { + const index = makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }); + renderPanel({ analysis, index }); + await openFilters(); + + await userEvent.click(notInTextFilter()); + + expect(listedAnalysisIds()).toEqual(['gone']); + }); + + it('shows how far reading the text has got while filtering for them', async () => { + const index = makeIndex({ status: 'loading', booksRead: 12, bookCount: 66 }); + renderPanel({ analysis, index }); + await openFilters(); + + await userEvent.click(notInTextFilter()); + + expect(screen.getByTestId('text-reading-status')).toHaveTextContent('12 of 66'); + expect(screen.queryByTestId('catalog-row')).not.toBeInTheDocument(); + }); + + it('says so when the text could not be read', async () => { + renderPanel({ analysis, index: makeIndex({ status: 'error' }) }); + await openFilters(); + + await userEvent.click(notInTextFilter()); + + expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-status', 'error'); + }); + + it('marks an analysis whose form the text lacks once it is read', () => { + renderPanel({ + analysis, + index: makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }), + }); + + expect(within(rowFor('gone')).getByTestId('catalog-row-not-in-text')).toBeInTheDocument(); + }); + + it('does not mark an analysis whose form the text holds', () => { + renderPanel({ + analysis, + index: makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }), + }); + + expect( + within(rowFor('kept')).queryByTestId('catalog-row-not-in-text'), + ).not.toBeInTheDocument(); + }); + + it('marks no analysis while the text is unread', () => { + renderPanel({ analysis }); + + expect(screen.queryByTestId('catalog-row-not-in-text')).not.toBeInTheDocument(); + }); + }); + describe('editing a row', () => { /** One analysis, shared by two tokens, so an edit here is visibly an edit to both. */ const SHARED: TextAnalysis = { diff --git a/src/__tests__/components/CatalogMergeModal.test.tsx b/src/__tests__/components/CatalogMergeModal.test.tsx index fc6e9276a..34ada537a 100644 --- a/src/__tests__/components/CatalogMergeModal.test.tsx +++ b/src/__tests__/components/CatalogMergeModal.test.tsx @@ -32,6 +32,7 @@ function row(analysisId: string, overrides: Partial = {}): CatalogRo return { analysisId, surfaceText: 'λόγος', + form: 'λόγος', gloss: '', glosses: gloss ? { [analysisLanguage]: gloss } : undefined, morphemes: [], diff --git a/src/__tests__/components/CatalogRowHeader.test.tsx b/src/__tests__/components/CatalogRowHeader.test.tsx index 6eb6f528c..dfc02468c 100644 --- a/src/__tests__/components/CatalogRowHeader.test.tsx +++ b/src/__tests__/components/CatalogRowHeader.test.tsx @@ -19,6 +19,7 @@ function row(overrides: Partial = {}): CatalogRow { return { analysisId: 'ta-1', surfaceText: 'λόγος', + form: 'λόγος', gloss: 'word', morphemes: [], usageCount: 0, @@ -38,6 +39,7 @@ function renderHeader(overrides: Partial usageCountInBookLabel: 'uses in Genesis', isExpanded: false, onToggle: jest.fn(), + isNotInText: false, isChecked: false, onCheckedChange: jest.fn(), localizedStrings: STRINGS, @@ -124,6 +126,20 @@ describe('CatalogRowHeader', () => { expect(screen.queryByTestId('catalog-row-stale-count')).not.toBeInTheDocument(); }); + it('marks an analysis whose form the text lacks', () => { + renderHeader({ isNotInText: true }); + + expect(screen.getByTestId('catalog-row-not-in-text')).toHaveTextContent( + '%interlinearizer_analysisCatalog_notInText%', + ); + }); + + it('leaves unmarked an analysis whose form the text holds', () => { + renderHeader(); + + expect(screen.queryByTestId('catalog-row-not-in-text')).not.toBeInTheDocument(); + }); + it('leaves the toggle unlabeled so that its own content names it', () => { renderHeader(); diff --git a/src/__tests__/components/ConcordanceIndexContext.test.tsx b/src/__tests__/components/ConcordanceIndexContext.test.tsx index 111c61d16..c36782315 100644 --- a/src/__tests__/components/ConcordanceIndexContext.test.tsx +++ b/src/__tests__/components/ConcordanceIndexContext.test.tsx @@ -17,7 +17,9 @@ const INDEX: ConcordanceIndex = { booksRead: 1, bookCount: 1, entries: [], + textForms: undefined, refresh: () => {}, + request: () => {}, }; describe('ConcordanceIndexProvider', () => { diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index 12efea216..e24b5ed68 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -17,6 +17,9 @@ import { buildConcordanceEntries, indexBook } from '../../utils/concordance'; import { defaultScrRef, FIXTURE_STAMPS, makeScrollGroupHook, makeVerseBook } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; +// How reading progress and failure read is the status's own concern. +jest.mock('../../components/TextReadingStatus'); + /** * The intersection-observer Jest stub exposes a helper for firing intersections on the global * object. Declared here so the windowing tests reach it without a type assertion. @@ -63,7 +66,9 @@ function makeIndex(overrides: Partial = {}): ConcordanceIndex booksRead: 2, bookCount: 2, entries: ENTRIES, + textForms: undefined, refresh: () => {}, + request: () => {}, ...overrides, }; } @@ -137,16 +142,14 @@ describe('ConcordancePanel', () => { it('shows how far reading the books has got while the index is built', () => { renderPanel({ index: makeIndex({ status: 'loading', booksRead: 12, bookCount: 66 }) }); - expect(screen.getByTestId('concordance-loading')).toHaveTextContent('Reading 12 of 66'); + expect(screen.getByTestId('text-reading-status')).toHaveTextContent('12 of 66'); expect(screen.queryByTestId('concordance-row')).not.toBeInTheDocument(); }); it('says so when the books could not be listed', () => { renderPanel({ index: makeIndex({ status: 'error' }) }); - expect(screen.getByTestId('concordance-error')).toHaveTextContent( - '%interlinearizer_concordance_error%', - ); + expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-status', 'error'); }); it('says so when the text has no words', () => { diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index d42b7e741..46d3be291 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -24,7 +24,10 @@ import InterlinearizerLoader, { UNDO_NOTIFICATION_DURATION_MS, } from '../../components/InterlinearizerLoader'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; -import useConcordanceIndex, { type ConcordanceIndex } from '../../hooks/useConcordanceIndex'; +import useConcordanceIndex, { + type ConcordanceIndex, + type UseConcordanceIndexArgs, +} from '../../hooks/useConcordanceIndex'; import useInterlinearizerBookData from '../../hooks/useInterlinearizerBookData'; import useLexiconRegistry from '../../hooks/useLexiconRegistry'; import useLostBoundaryDismissal from '../../hooks/useLostBoundaryDismissal'; @@ -594,6 +597,19 @@ function renderLoader( return { ...result, updateWebViewDefinition }; } +/** Has the index of the source text report itself unread, as it is until something asks for it. */ +function mockUnreadConcordanceIndex(): void { + jest.mocked(useConcordanceIndex).mockReturnValue({ + status: 'idle', + booksRead: 0, + bookCount: 0, + entries: [], + textForms: undefined, + refresh: jest.fn(), + request: jest.fn(), + }); +} + /** Configures useInterlinearizerBookData to return the given state. */ function mockBookData( overrides: Partial<{ @@ -774,6 +790,7 @@ describe('InterlinearizerLoader', () => { }); mockSettings(); mockSourceShortName('WEB'); + mockUnreadConcordanceIndex(); }); it('shows nav controls when interface mode is power', async () => { @@ -4010,7 +4027,9 @@ describe('InterlinearizerLoader', () => { booksRead: 0, bookCount: 0, entries: [], + textForms: undefined, refresh: jest.fn(), + request: jest.fn(), }); }); @@ -4123,13 +4142,15 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('tab-toolbar-concordance')); - expect(useConcordanceIndex).toHaveBeenLastCalledWith({ - projectId: testProjectId, - writingSystem: 'hbo', - liveBook: GEN_1_1_BOOK, - enabled: true, - shown: true, - }); + expect(useConcordanceIndex).toHaveBeenLastCalledWith( + expect.objectContaining({ + projectId: testProjectId, + writingSystem: 'hbo', + liveBook: GEN_1_1_BOOK, + enabled: true, + shown: true, + }), + ); }); it('keeps the concordance index across a wipe that replaces the draft', async () => { @@ -4139,7 +4160,9 @@ describe('InterlinearizerLoader', () => { booksRead: 0, bookCount: 0, entries: [], + textForms: undefined, refresh: jest.fn(), + request: jest.fn(), }; jest.mocked(useConcordanceIndex).mockImplementation(() => { // Held in state so a remount of whatever calls the hook shows up as a fresh count. @@ -4878,6 +4901,7 @@ function prepareStoreProbeTest(): void { mockLostBoundaries([]); mockStaleAnalyses(); mockProjectBookIds(undefined); + mockUnreadConcordanceIndex(); mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); jest .mocked(useData) @@ -4978,6 +5002,183 @@ describe('analysis store lifetime', () => { }); }); +describe('whole-text re-anchoring', () => { + beforeEach(prepareStoreProbeTest); + + afterEach(() => { + mountStoreProbe = false; + }); + + /** The arguments the loader last handed the index of the source text. */ + function indexArgs(): UseConcordanceIndexArgs { + const { calls } = jest.mocked(useConcordanceIndex).mock; + return calls[calls.length - 1][0]; + } + + /** A one-verse book reading "Alpha beta.", where "beta" begins at offset 6. */ + function alphaBetaBook(bookCode: string): Book { + return { + id: bookCode, + bookRef: bookCode, + textVersion: 'v1', + duplicateVerseIds: [], + segments: [ + makeSegment(`${bookCode} 1:1`, 'Alpha beta.', [ + makeWordToken(`${bookCode} 1:1:0`, 'Alpha'), + makeWordToken(`${bookCode} 1:1:6`, 'beta', 6), + ]), + ], + }; + } + + /** Loads a draft approving "beta" at `tokenRef`, as written against "Al beta.". */ + async function renderWithApprovalAt(tokenRef: string): Promise { + const analysis = analysisApprovingAt(tokenRef, 'beta'); + mockSendCommand.mockResolvedValue(JSON.stringify({ ...emptyDraft(testProjectId), analysis })); + await act(async () => renderLoader()); + } + + it('re-anchors the draft to a book the text read hands on', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + + act(() => indexArgs().onBookRead?.(alphaBetaBook('EXO'))); + + expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('EXO 1:1:6'); + }); + + it('leaves the loaded book to its live text rather than a reading of it', async () => { + mockBookData({ book: alphaBetaBook('GEN') }); + await renderWithApprovalAt('GEN 1:1:3'); + const olderReading: Book = { + ...alphaBetaBook('GEN'), + segments: [ + makeSegment('GEN 1:1', 'Al beta.', [ + makeWordToken('GEN 1:1:0', 'Al'), + makeWordToken('GEN 1:1:3', 'beta', 3), + ]), + ], + }; + + act(() => indexArgs().onBookRead?.(olderReading)); + + expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('GEN 1:1:6'); + }); + + it('stales the approvals in a book the read text lacks', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + + act(() => indexArgs().onTextRead?.(['GEN'])); + + expect(probeAnalysis?.tokenAnalysisLinks[0].status).toBe('stale'); + }); + + it('leaves the approvals in a book the read text holds', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + + act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); + + expect(probeAnalysis?.tokenAnalysisLinks[0].status).toBe('approved'); + }); + + /** Opens the catalog and its filter controls. */ + async function openCatalogFilters(): Promise { + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + await userEvent.click(screen.getByTestId('catalog-filters-button')); + } + + it('says stale places are found only in opened books until the text is read', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + + await openCatalogFilters(); + + expect( + screen.getByRole('checkbox', { name: '%interlinearizer_analysisCatalog_filter_stale%' }), + ).toBeInTheDocument(); + }); + + it('says stale places are found in every book once the text is read', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); + + await openCatalogFilters(); + + expect( + screen.getByRole('checkbox', { + name: '%interlinearizer_analysisCatalog_filter_staleEverywhere%', + }), + ).toBeInTheDocument(); + }); + + it('says stale places are found only in opened books once the draft is replaced', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); + await userEvent.click(screen.getByTestId('tab-toolbar-wipe')); + await userEvent.click(screen.getByTestId('wipe-confirm-book')); + + await openCatalogFilters(); + + expect( + screen.getByRole('checkbox', { name: '%interlinearizer_analysisCatalog_filter_stale%' }), + ).toBeInTheDocument(); + }); + + it('reads the text again once the draft is replaced', async () => { + await renderWithApprovalAt('EXO 1:1:3'); + const { readKey } = indexArgs(); + + await userEvent.click(screen.getByTestId('tab-toolbar-wipe')); + await userEvent.click(screen.getByTestId('wipe-confirm-book')); + + expect(indexArgs().readKey).not.toBe(readKey); + }); + + describe('while an import is shown', () => { + /** Shows a stored import in place of a draft approving "beta" at EXO 1:1:3. */ + async function renderImportBesideDraft(): Promise { + const analysis = analysisApprovingAt('EXO 1:1:3', 'beta'); + mockSendCommand.mockImplementation(async (...args) => + JSON.stringify( + args[0] === 'interlinearizer.getProject' + ? { ...STUB_IMPORT_PROJECT, analysis: emptyAnalysis() } + : { ...emptyDraft(testProjectId), analysis }, + ), + ); + await act(async () => + renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }), + ); + await screen.findByTestId('pt9-import-banner'); + } + + /** Lets any write of the draft past its debounce, then whether one was made. */ + function draftWasSaved(): boolean { + act(() => jest.advanceTimersByTime(1000)); + return mockSendCommand.mock.calls.some( + ([command]) => command === 'interlinearizer.saveDraft', + ); + } + + afterEach(() => jest.useRealTimers()); + + it('re-anchors the draft to no book the text read hands on', async () => { + await renderImportBesideDraft(); + jest.useFakeTimers(); + + act(() => indexArgs().onBookRead?.(alphaBetaBook('EXO'))); + + expect(draftWasSaved()).toBe(false); + }); + + it('stales nothing in the draft for a book the read text lacks', async () => { + await renderImportBesideDraft(); + jest.useFakeTimers(); + + act(() => indexArgs().onTextRead?.(['GEN'])); + + expect(draftWasSaved()).toBe(false); + }); + }); +}); + describe('undo and redo', () => { beforeEach(() => { prepareStoreProbeTest(); diff --git a/src/__tests__/components/TextReadingStatus.test.tsx b/src/__tests__/components/TextReadingStatus.test.tsx new file mode 100644 index 000000000..2d1cd19ae --- /dev/null +++ b/src/__tests__/components/TextReadingStatus.test.tsx @@ -0,0 +1,38 @@ +/// +/// + +import { render, screen } from '@testing-library/react'; +import TextReadingStatus from '../../components/TextReadingStatus'; + +const localizedStrings = { + '%interlinearizer_concordance_loading%': 'Reading {read} of {total}', + '%interlinearizer_concordance_error%': 'The text could not be read.', +}; + +describe('TextReadingStatus', () => { + it('shows how many books have been read while the text is read', () => { + render( + , + ); + + expect(screen.getByTestId('panel-loading')).toHaveTextContent('Reading 12 of 66'); + expect(screen.queryByTestId('panel-error')).not.toBeInTheDocument(); + }); + + it('says so when the text could not be read', () => { + render( + , + ); + + expect(screen.getByTestId('panel-error')).toHaveTextContent('The text could not be read.'); + expect(screen.queryByTestId('panel-loading')).not.toBeInTheDocument(); + }); +}); diff --git a/src/__tests__/hooks/useConcordanceIndex.test.ts b/src/__tests__/hooks/useConcordanceIndex.test.ts index 4fb3d2d87..ba0867de7 100644 --- a/src/__tests__/hooks/useConcordanceIndex.test.ts +++ b/src/__tests__/hooks/useConcordanceIndex.test.ts @@ -69,7 +69,7 @@ beforeEach(() => { jest.mocked(indexBook).mockImplementation((b) => ({ book: b.bookRef, textVersion: b.textVersion, - occurrencesByForm: new Map(), + occurrencesByForm: new Map([[`${b.bookRef}@${b.textVersion}`, []]]), spellingCountsByForm: new Map(), })); jest.mocked(buildConcordanceEntries).mockImplementation((books) => @@ -97,6 +97,16 @@ describe('useConcordanceIndex', () => { expect(mockPdpGet).not.toHaveBeenCalled(); }); + it('reads the books once they are requested', async () => { + serveProject('1', async (id) => usj(id, 'v1')); + const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, enabled: false })); + + act(() => result.current.request()); + + await waitFor(() => expect(result.current.status).toBe('ready')); + expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v1']); + }); + it('reads every book the project marks present', async () => { const getBookUSJ = serveProject('1010', async (id) => usj(id, 'v1')); @@ -152,6 +162,42 @@ describe('useConcordanceIndex', () => { expect(builtFrom(result.current.entries)).toEqual(['EXO@v1']); }); + it('hands on each book it reads', async () => { + serveProject('11', async (id) => usj(id, 'v1')); + const onBookRead = jest.fn(); + + renderHook(() => useConcordanceIndex({ ...baseArgs, onBookRead })); + + await waitFor(() => + expect(onBookRead.mock.calls.map(([read]) => read)).toEqual([ + book('GEN', 'v1'), + book('EXO', 'v1'), + ]), + ); + }); + + it('reports the books it read text for once every book is in', async () => { + serveProject('111', async (id) => (id === 'EXO' ? undefined : usj(id, 'v1'))); + const onTextRead = jest.fn(); + + renderHook(() => useConcordanceIndex({ ...baseArgs, onTextRead })); + + await waitFor(() => expect(onTextRead).toHaveBeenCalledWith(['GEN', 'LEV'])); + }); + + it('reports no reading of the text while a book of it failed to read', async () => { + serveProject('11', async (id) => { + if (id === 'GEN') throw new Error('unreadable'); + return usj(id, 'v1'); + }); + const onTextRead = jest.fn(); + + const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, onTextRead })); + + await waitFor(() => expect(result.current.status).toBe('ready')); + expect(onTextRead).not.toHaveBeenCalled(); + }); + it('reports an error when no book could be read', async () => { serveProject('11', async (id) => { if (id === 'GEN') throw new Error('unreadable'); @@ -194,6 +240,17 @@ describe('useConcordanceIndex', () => { expect(builtFrom(result.current.entries)).toEqual(['EXO@v1', 'GEN@v2']); }); + it('serves the forms of every book, the live one in place of its reading', async () => { + serveProject('11', async (id) => usj(id, 'v1')); + + const { result } = renderHook(() => + useConcordanceIndex({ ...baseArgs, liveBook: book('GEN', 'v2') }), + ); + + await waitFor(() => expect(result.current.status).toBe('ready')); + expect([...(result.current.textForms ?? [])].sort()).toEqual(['EXO@v1', 'GEN@v2']); + }); + it('keeps the last live version of a book the editor moves off', async () => { serveProject('11', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( @@ -222,6 +279,19 @@ describe('useConcordanceIndex', () => { expect(builtFrom(result.current.entries)).toEqual(['GEN@v1']); }); + it('takes live-book edits into the forms while the concordance is hidden', async () => { + serveProject('1', async (id) => usj(id, 'v1')); + const { result, rerender } = renderHook( + (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), + { initialProps: { ...baseArgs, shown: false } }, + ); + await waitFor(() => expect(result.current.status).toBe('ready')); + + rerender({ ...baseArgs, shown: false, liveBook: book('GEN', 'v2') }); + + expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v2']); + }); + it('takes in edits held back while hidden once the concordance is shown', async () => { serveProject('1', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( @@ -286,6 +356,50 @@ describe('useConcordanceIndex', () => { expect(builtFrom(result.current.entries)).toEqual(['EXO@v1', 'GEN@v2']); }); + it('hands on no book from a read abandoned by a refresh', async () => { + const firstGenesis = deferred(); + let genesisReads = 0; + serveProject('1', () => { + genesisReads += 1; + return genesisReads === 1 ? firstGenesis.promise : Promise.resolve(usj('GEN', 'v2')); + }); + const onBookRead = jest.fn(); + const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, onBookRead })); + await waitFor(() => expect(genesisReads).toBe(1)); + act(() => result.current.refresh()); + await waitFor(() => expect(result.current.status).toBe('ready')); + + await act(async () => firstGenesis.resolve(usj('GEN', 'stale'))); + + expect(onBookRead.mock.calls.map(([read]) => read)).toEqual([book('GEN', 'v2')]); + }); + + it('reads every book again when its read key changes', async () => { + let version = 'v1'; + serveProject('1', async (id) => usj(id, version)); + const { result, rerender } = renderHook( + (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), + { initialProps: { ...baseArgs, readKey: 1 } }, + ); + await waitFor(() => expect(result.current.status).toBe('ready')); + version = 'v2'; + + rerender({ ...baseArgs, readKey: 2 }); + + await waitFor(() => expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v2'])); + }); + + it('reads nothing when its read key changes before the index is wanted', () => { + const getBookUSJ = serveProject('1', async (id) => usj(id, 'v1')); + const { rerender } = renderHook((args: UseConcordanceIndexArgs) => useConcordanceIndex(args), { + initialProps: { ...baseArgs, enabled: false, readKey: 1 }, + }); + + rerender({ ...baseArgs, enabled: false, readKey: 2 }); + + expect(getBookUSJ).not.toHaveBeenCalled(); + }); + it('forgets live versions of other books on refresh when no book is live', async () => { let version = 'v1'; serveProject('1', async (id) => usj(id, version)); diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 47a859bb2..b5f6b4b14 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -5885,6 +5885,7 @@ describe('analysis-keyed reducers', () => { return { analysisId, surfaceText: 'word', + form: 'word', gloss: '', morphemes: payload?.morphemes ?? [], usageCount: 1, diff --git a/src/__tests__/utils/analysis-query.test.ts b/src/__tests__/utils/analysis-query.test.ts index dc6ff844b..9a9a445e8 100644 --- a/src/__tests__/utils/analysis-query.test.ts +++ b/src/__tests__/utils/analysis-query.test.ts @@ -891,6 +891,34 @@ describe('applyCatalogQuery filters', () => { expect(applyCatalogQuery(rows, query)).toEqual([]); }); + it('keeps only rows whose form the text lacks when filtering for not in the text', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'Abraham' }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'Sarai' }, + ], + }; + const rows = buildCatalogRows(analysis, scope); + const query = makeQuery({ + filters: { notInText: true }, + textForms: new Set(['abraham', 'sarah']), + }); + + expect(applyCatalogQuery(rows, query).map((r) => r.analysisId)).toEqual(['ta-2']); + }); + + it('keeps no row as not in the text while the text is unread', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'Sarai' }], + }; + const rows = buildCatalogRows(analysis, scope); + const query = makeQuery({ filters: { notInText: true } }); + + expect(applyCatalogQuery(rows, query)).toEqual([]); + }); + // ta-2 is glossed, just not in the scope's language, so an any-language check would drop it. it('keeps a row glossed only in another language when filtering for a missing gloss', () => { const analysis: TextAnalysis = { @@ -1189,6 +1217,12 @@ describe('reconcileFilters', () => { expect(filters.morphemes).toBe('has'); }); + it('keeps the not-in-the-text filter when another selection is withdrawn', () => { + const filters = reconcileFilters({ books: ['EXO'], notInText: true }, { books: undefined }); + + expect(filters.notInText).toBe(true); + }); + it('drops a feature selection whose name lost its facet', () => { const filters = reconcileFilters( { features: { case: ['nom'], number: ['sg'] } }, diff --git a/src/__tests__/utils/merge-content.test.ts b/src/__tests__/utils/merge-content.test.ts index 77ea0ad52..a168b49f1 100644 --- a/src/__tests__/utils/merge-content.test.ts +++ b/src/__tests__/utils/merge-content.test.ts @@ -28,6 +28,7 @@ function row(analysisId: string, overrides: Partial = {}): CatalogRo return { analysisId, surfaceText: 'λόγος', + form: 'λόγος', gloss: '', glosses: gloss ? { [analysisLanguage]: gloss } : undefined, morphemes: [], diff --git a/src/__tests__/utils/reanchor-draft.test.ts b/src/__tests__/utils/reanchor-draft.test.ts index be2d3259e..195e72970 100644 --- a/src/__tests__/utils/reanchor-draft.test.ts +++ b/src/__tests__/utils/reanchor-draft.test.ts @@ -2,7 +2,7 @@ import type { TextAnalysis } from 'interlinearizer'; import { emptyAnalysis } from '../../types/empty-factories'; -import { reanchorDraftToBook } from '../../utils/reanchor-draft'; +import { reanchorDraftToBook, reanchorDraftToMissingBook } from '../../utils/reanchor-draft'; import { FIXTURE_STAMPS, makeVerseBook } from '../test-helpers'; /** An analysis approving `gloss` on the sole occurrence of `surfaceText` in a one-verse `text`. */ @@ -96,3 +96,43 @@ describe('reanchorDraftToBook', () => { expect(reanchored.segmentation).toBe(segmentation); }); }); + +describe('reanchorDraftToMissingBook', () => { + it('stales an approval in the book', () => { + const content = { analysis: glossedOn('alpha beta', 'beta', 'bêta'), segmentation: undefined }; + + const reanchored = reanchorDraftToMissingBook('GEN')(content); + + expect(reanchored.analysis.tokenAnalysisLinks[0].status).toBe('stale'); + }); + + it('stales a free translation in the book', () => { + const content = { + analysis: { + ...emptyAnalysis(), + segmentAnalyses: [{ ...FIXTURE_STAMPS, id: 'sa-1', surfaceText: 'alpha beta' }], + segmentAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + analysisId: 'sa-1', + status: 'approved' as const, + segmentId: 'GEN 1:1', + }, + ], + }, + segmentation: undefined, + }; + + const reanchored = reanchorDraftToMissingBook('GEN')(content); + + expect(reanchored.analysis.segmentAnalysisLinks[0].status).toBe('stale'); + }); + + it("leaves another book's approvals approved", () => { + const content = { analysis: glossedOn('alpha beta', 'beta', 'bêta'), segmentation: undefined }; + + const reanchored = reanchorDraftToMissingBook('EXO')(content); + + expect(reanchored.analysis.tokenAnalysisLinks[0].status).toBe('approved'); + }); +}); diff --git a/src/__tests__/utils/stale-free-translations.test.ts b/src/__tests__/utils/stale-free-translations.test.ts index 36ee07db3..9b1b33cf6 100644 --- a/src/__tests__/utils/stale-free-translations.test.ts +++ b/src/__tests__/utils/stale-free-translations.test.ts @@ -109,12 +109,54 @@ describe('placeStaleFreeTranslations', () => { expect(placed([stale('GEN 1:1/s1')], book)).toEqual({ 'GEN 1:1': ['sa GEN 1:1/s1'] }); }); - it('shows a translation of a heading whose verse the book no longer holds nowhere', () => { - expect(placed([stale('GEN 1:9/s1')])).toEqual({}); + it('files a translation of a deleted verse under the segment ending the verse before it', () => { + const book = resegmentBook( + makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning God created' }, + { sid: 'GEN 1:3', text: 'and God said' }, + ]), + { removedVerseStarts: [], addedStarts: [{ tokenRef: 'GEN 1:1:17', surfaceText: 'God' }] }, + ); + + expect(placed([stale('GEN 1:2')], book)).toEqual({ 'GEN 1:1:17': ['sa GEN 1:2'] }); + }); + + it('files a translation of a deleted verse under the last segment of an earlier chapter', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:3', text: 'and God said' }, + { sid: 'GEN 2:1', text: 'Thus the heavens' }, + ]); + + expect(placed([stale('GEN 1:9')], book)).toEqual({ 'GEN 1:3': ['sa GEN 1:9'] }); + }); + + it('files a translation of a deleted verse ahead of every other under the first segment', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:2', text: 'and the earth was void' }, + { sid: 'GEN 1:3', text: 'and God said' }, + ]); + + expect(placed([stale('GEN 1:1')], book)).toEqual({ 'GEN 1:2': ['sa GEN 1:1'] }); + }); + + it('files a translation of a heading whose verse was deleted under the segment before it', () => { + expect(placed([stale('GEN 1:9/s1')])).toEqual({ 'GEN 1:3': ['sa GEN 1:9/s1'] }); + }); + + it("files a deleted verse's translation after the translations of the segment it falls under", () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:3', text: 'and God said' }, + ]); + + expect(placed([stale('GEN 1:2'), stale('GEN 1:1')], book)).toEqual({ + 'GEN 1:1': ['sa GEN 1:1', 'sa GEN 1:2'], + }); }); - it('shows a translation of a verse the book no longer holds nowhere', () => { - expect(placed([stale('GEN 1:9')])).toEqual({}); + it('shows a translation nowhere in a book holding no text', () => { + expect(placed([stale('GEN 1:1')], makeVerseBook([]))).toEqual({}); }); it('shows a translation of another book nowhere', () => { diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index a860cbfa5..0c3f54e64 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -32,6 +32,8 @@ import CatalogList from './CatalogList'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import CatalogSelectionBar, { SELECTION_BAR_STRING_KEYS } from './CatalogSelectionBar'; import SidePanelTabs, { SIDE_PANEL_TAB_STRING_KEYS } from './SidePanelTabs'; +import { useConcordanceIndexContext } from './ConcordanceIndexContext'; +import TextReadingStatus, { TEXT_READING_STRING_KEYS } from './TextReadingStatus'; import { useInterlinearNav } from './InterlinearNavContext'; import useDraftDiscardGate from '../hooks/useDraftDiscardGate'; import useRowSelection from '../hooks/useRowSelection'; @@ -108,6 +110,7 @@ const STRING_KEYS = [ '%interlinearizer_analysisCatalog_mergedEmpty%', '%interlinearizer_analysisCatalog_usageCountInBook%', '%interlinearizer_analysisCatalog_noMatches%', + ...TEXT_READING_STRING_KEYS, ...QUERY_CONTROL_STRING_KEYS, ...ROW_STRING_KEYS, ...SELECTION_BAR_STRING_KEYS, @@ -169,6 +172,8 @@ type AnalysisCatalogPanelProps = Readonly<{ showMorphology: boolean; /** BCP 47 tag of the source text, so surface forms collate by their own language. */ sourceLanguageTag: string; + /** Whether every book has been checked for stale places, rather than only the books opened. */ + staleCoversDraft: boolean; }>; /** @@ -194,6 +199,7 @@ export default function AnalysisCatalogPanel({ liveSurfaceText, showMorphology, sourceLanguageTag, + staleCoversDraft, }: AnalysisCatalogPanelProps) { const [localizedStrings] = useLocalizedStrings(STRING_KEYS); const analysisLanguage = useAnalysisLanguage(); @@ -216,6 +222,18 @@ export default function AnalysisCatalogPanel({ */ const [chosenFilters, setFilters] = useState({}); + const textIndex = useConcordanceIndexContext(); + const { request: requestText, textForms } = textIndex; + + /** Records the reader's filters, asking for the text to be read once one needs it. */ + const handleFiltersChange = useCallback( + (next: CatalogFilters) => { + if (next.notInText || next.stale) requestText(); + setFilters(next); + }, + [requestText], + ); + // Each rebuilt only when its own tag changes: the query around them turns over on every keystroke // in the search box, and a collator is expensive enough to be worth not rebuilding that often. const surfaceCollator = useMemo(() => collatorForTag(sourceLanguageTag), [sourceLanguageTag]); @@ -243,12 +261,15 @@ export default function AnalysisCatalogPanel({ /** How the listing is narrowed and ordered, from the controls above the list. */ const query = useMemo( - () => ({ search, sort, filters, surfaceCollator, glossCollator }), - [search, sort, filters, surfaceCollator, glossCollator], + () => ({ search, sort, filters, surfaceCollator, glossCollator, textForms }), + [search, sort, filters, surfaceCollator, glossCollator, textForms], ); const rows = useMemo(() => applyCatalogQuery(catalogRows, query), [catalogRows, query]); + /** Whether the listing waits on the text, a filter needing it before it is read. */ + const isAwaitingText = filters.notInText && !textForms; + /** * The current book's name key, asked for separately from {@link STRING_KEYS} so that changing book * re-resolves this alone rather than every string the panel shows. @@ -1011,12 +1032,13 @@ export default function AnalysisCatalogPanel({ facets={facets} filters={filters} localizedStrings={localizedStrings} - onFiltersChange={setFilters} + onFiltersChange={handleFiltersChange} onSearchChange={setSearch} onSortChange={setSort} search={search} showMorphology={showMorphology} sort={sort} + staleCoversDraft={staleCoversDraft} /> )} @@ -1049,52 +1071,64 @@ export default function AnalysisCatalogPanel({ /> )} - {rows.length === 0 ? ( - // Two ways to have nothing to list, and they call for different answers: a draft that has - // recorded nothing yet, and a query that kept none of what it did. Telling a reader the - // draft is empty when they have merely mistyped would send them looking for lost work. - - ) : ( - - {windowRows.map((row) => ( - - ))} - )} + {!isAwaitingText && + (rows.length === 0 ? ( + // Two ways to have nothing to list, and they call for different answers: a draft that has + // recorded nothing yet, and a query that kept none of what it did. Telling a reader the + // draft is empty when they have merely mistyped would send them looking for lost work. + + ) : ( + + {windowRows.map((row) => ( + + ))} + + ))} + {/* Both modals are mounted against what they still have to act on rather than against the id alone, so a listing that turns over beneath one — an edit made in the view beside the diff --git a/src/components/CatalogFilterPopover.tsx b/src/components/CatalogFilterPopover.tsx index c50c38872..95b7c067d 100644 --- a/src/components/CatalogFilterPopover.tsx +++ b/src/components/CatalogFilterPopover.tsx @@ -46,6 +46,8 @@ export const FILTER_STRING_KEYS = [ '%interlinearizer_analysisCatalog_filter_morphemes_lacks%', '%interlinearizer_analysisCatalog_filter_zeroUsages%', '%interlinearizer_analysisCatalog_filter_stale%', + '%interlinearizer_analysisCatalog_filter_staleEverywhere%', + '%interlinearizer_analysisCatalog_filter_notInText%', ...Object.values(CONFIDENCE_LABEL_KEYS), ] as const satisfies `%${string}%`[]; @@ -244,6 +246,8 @@ type CatalogFilterPopoverProps = Readonly<{ onFiltersChange: (filters: CatalogFilters) => void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ showMorphology: boolean; + /** Whether every book has been checked for stale places, rather than only the books opened. */ + staleCoversDraft: boolean; /** What the language the missing-gloss filter asks about is called, as its label names it. */ analysisLanguageName: string; /** Resolved localizations covering at least {@link FILTER_STRING_KEYS}. */ @@ -266,6 +270,7 @@ export default function CatalogFilterPopover({ filters, onFiltersChange, showMorphology, + staleCoversDraft, analysisLanguageName, localizedStrings, }: CatalogFilterPopoverProps) { @@ -288,8 +293,13 @@ export default function CatalogFilterPopover({ filters.confidence, ...Object.values(filters.features ?? {}), ].filter((selected) => selected?.length).length + - [filters.missingGloss, filters.morphemes, filters.zeroUsages, filters.stale].filter(Boolean) - .length; + [ + filters.missingGloss, + filters.morphemes, + filters.zeroUsages, + filters.stale, + filters.notInText, + ].filter(Boolean).length; return ( @@ -428,9 +438,19 @@ export default function CatalogFilterPopover({ onFiltersChange({ ...filters, stale })} /> + + onFiltersChange({ ...filters, notInText })} + /> )} diff --git a/src/components/CatalogList.tsx b/src/components/CatalogList.tsx index da478e07f..349778d04 100644 --- a/src/components/CatalogList.tsx +++ b/src/components/CatalogList.tsx @@ -17,7 +17,7 @@ type CatalogListProps = Readonly<{ export default function CatalogList({ scrollRef, sentinelRef, children }: CatalogListProps) { return (
    {children} diff --git a/src/components/CatalogQueryControls.tsx b/src/components/CatalogQueryControls.tsx index 30a1d3b27..82e541852 100644 --- a/src/components/CatalogQueryControls.tsx +++ b/src/components/CatalogQueryControls.tsx @@ -50,6 +50,8 @@ type CatalogQueryControlsProps = Readonly<{ onFiltersChange: (filters: CatalogFilters) => void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ showMorphology: boolean; + /** Whether every book has been checked for stale places, rather than only the books opened. */ + staleCoversDraft: boolean; /** What the language the missing-gloss filter asks about is called. */ analysisLanguageName: string; /** What the current book is called, as prose, for the per-book sort to be named after. */ @@ -75,6 +77,7 @@ export default function CatalogQueryControls({ filters, onFiltersChange, showMorphology, + staleCoversDraft, analysisLanguageName, currentBookName, localizedStrings, @@ -125,6 +128,7 @@ export default function CatalogQueryControls({ localizedStrings={localizedStrings} onFiltersChange={onFiltersChange} showMorphology={showMorphology} + staleCoversDraft={staleCoversDraft} /> diff --git a/src/components/CatalogRowHeader.tsx b/src/components/CatalogRowHeader.tsx index 483b598c0..fb2233306 100644 --- a/src/components/CatalogRowHeader.tsx +++ b/src/components/CatalogRowHeader.tsx @@ -1,4 +1,4 @@ -import { ChevronDown, ChevronRight, TriangleAlert } from 'lucide-react'; +import { ChevronDown, ChevronRight, SearchX, TriangleAlert } from 'lucide-react'; import { Button, Checkbox, @@ -18,6 +18,7 @@ export const ROW_HEADER_STRING_KEYS = [ '%interlinearizer_analysisCatalog_noGloss%', '%interlinearizer_analysisCatalog_usageCount%', '%interlinearizer_analysisCatalog_staleCount%', + '%interlinearizer_analysisCatalog_notInText%', ] as const satisfies `%${string}%`[]; /** Props for {@link CatalogRowHeader}. */ @@ -30,6 +31,8 @@ type CatalogRowHeaderProps = Readonly<{ isExpanded: boolean; /** Shows the row's detail, or hides it. */ onToggle: () => void; + /** Whether the source text, as last read, holds this analysis's form nowhere. */ + isNotInText: boolean; /** Whether the row is among those a bulk action applies to. */ isChecked: boolean; /** @@ -50,12 +53,14 @@ export default function CatalogRowHeader({ usageCountInBookLabel, isExpanded, onToggle, + isNotInText, isChecked, onCheckedChange, localizedStrings, }: CatalogRowHeaderProps) { const usageCountLabel = localizedStrings['%interlinearizer_analysisCatalog_usageCount%']; const staleCountLabel = localizedStrings['%interlinearizer_analysisCatalog_staleCount%']; + const notInTextLabel = localizedStrings['%interlinearizer_analysisCatalog_notInText%']; // This is visible cell text, so blanking an unresolved key would empty the gloss column. The em // dash reads as "no gloss" in any language and stands in until the lookup lands. @@ -165,6 +170,18 @@ export default function CatalogRowHeader({ )} + {isNotInText && ( + + {/* Placed in the last column, since the stale count before it may be absent. */} + + + {` ${notInTextLabel}`} + + + )} ); diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index bd35a8f01..c0b5c8c20 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -40,6 +40,8 @@ type CatalogRowViewProps = Readonly<{ * analysis, which no bulk action applies to. */ onCheckedChange?: (analysisId: string, checked: boolean) => void; + /** Whether the source text, as last read, holds this analysis's form nowhere. */ + isNotInText: boolean; /** Jumps the interlinear view to one of this analysis's usages. */ onUsageSelect: (analysisId: string, usage: CatalogUsage) => void; /** Moves the interlinear view to the verse of a place this analysis went stale at. */ @@ -111,6 +113,7 @@ function CatalogRowView({ isSelected, isChecked, onCheckedChange, + isNotInText, onUsageSelect, onStaleSelect, onStaleDiscard, @@ -210,6 +213,7 @@ function CatalogRowView({ - ); - } else if (index.status !== 'ready') { - body = ( -

    - - {formatReplacementString(localizedStrings['%interlinearizer_concordance_loading%'], { - read: index.booksRead, - total: index.bookCount, - })} -

    + ); } else if (index.entries.length === 0) { body = ( diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 4dc49851d..314be2180 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -29,7 +29,7 @@ import { useTransition, } from 'react'; import type { ComponentProps, ReactNode, RefObject } from 'react'; -import type { Segment, TextAnalysis } from 'interlinearizer'; +import type { Book, Segment, TextAnalysis } from 'interlinearizer'; import type { Pt9InterlinearProjectManifest } from 'platform-scripture'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import useUndoRedoKeys from '../hooks/useUndoRedoKeys'; @@ -49,7 +49,8 @@ import { splitSegmentBefore, unmergeableVerseStarts, } from '../utils/segmentation'; -import { reanchorDraftToBook } from '../utils/reanchor-draft'; +import { booksLinkedIn } from '../utils/analysis-book'; +import { reanchorDraftToBook, reanchorDraftToMissingBook } from '../utils/reanchor-draft'; import { isInterlinearProjectSummary, isTextAnalysis, isWordToken } from '../types/type-guards'; import { isPt9ImportReport, isPt9UnreadableFileList } from '../converters/pt9'; import { toProjectSummary } from '../types/interlinear-project-summary'; @@ -695,6 +696,52 @@ function InterlinearizerLoaderInner({ reanchorBook(reanchor.bookCode, reanchor.pass); }, [reanchorBook, reanchor, isImportView, isDraftLoading, segmentationVersion, draftVersion]); + /** The book the view has loaded, which re-anchors to its own live text rather than to a reading. */ + const loadedBookCodeRef = useRef(verseBook?.bookRef); + loadedBookCodeRef.current = verseBook?.bookRef; + + /** + * The version of the draft a reading of the whole text re-anchors, `undefined` while it is + * loading or an import is shown in its place; each change reads the text again. + */ + const readingTarget = isImportView || isDraftLoading ? undefined : draftVersion; + const readingTargetRef = useRef(readingTarget); + readingTargetRef.current = readingTarget; + + /** The draft version every book was last re-anchored for. */ + const [textReanchoredFor, setTextReanchoredFor] = useState(); + const staleCoversDraft = readingTarget !== undefined && textReanchoredFor === readingTarget; + + /** Re-anchors the draft to a book the whole-text read hands on, where it has anything there. */ + const handleBookRead = useCallback( + (read: Book) => { + if (read.bookRef === loadedBookCodeRef.current) return; + const analysis = + readingTargetRef.current === undefined ? undefined : getDraftSnapshot()?.analysis; + if (!analysis || !booksLinkedIn(analysis).has(read.bookRef)) return; + reanchorBook(read.bookRef, reanchorDraftToBook(read)); + }, + [getDraftSnapshot, reanchorBook], + ); + + /** + * Re-anchors the draft to the absence of every book it has anything in that the text lacks, + * completing its re-anchoring to the whole text. + */ + const handleTextRead = useCallback( + (bookIds: readonly string[]) => { + const target = readingTargetRef.current; + const analysis = target === undefined ? undefined : getDraftSnapshot()?.analysis; + if (!analysis) return; + const present = new Set(bookIds); + booksLinkedIn(analysis).forEach((bookCode) => { + if (!present.has(bookCode)) reanchorBook(bookCode, reanchorDraftToMissingBook(bookCode)); + }); + setTextReanchoredFor(target); + }, + [getDraftSnapshot, reanchorBook], + ); + /** * The book the views render: the verse-tokenized book re-grouped into the user's custom segments. * Identical (by reference) to `verseBook` when no custom boundaries are set in it, so the common @@ -880,7 +927,10 @@ function InterlinearizerLoaderInner({ DEFAULT_SIDE_PANEL_LAYOUT, ); - /** Whether the concordance has been shown in this tab, which is what starts the text being read. */ + /** + * Whether the concordance has been shown in this tab, one of the things that starts the text + * being read. + */ const [concordanceWanted, setConcordanceWanted] = useState(sidePanel === 'concordance'); if (sidePanel === 'concordance' && !concordanceWanted) setConcordanceWanted(true); @@ -1728,6 +1778,7 @@ function InterlinearizerLoaderInner({ onShowConcordance={handleShowConcordance} showMorphology={showMorphology} sourceLanguageTag={writingSystem} + staleCoversDraft={staleCoversDraft} /> ) : ( ; + /** Prefixes the `-loading` and `-error` test ids the status is found by. */ + idPrefix: string; + /** Resolved localizations covering at least {@link TEXT_READING_STRING_KEYS}. */ + localizedStrings: LanguageStrings; +}>; + +/** How far reading the source text has got, or that it could not be read. */ +export default function TextReadingStatus({ + index, + idPrefix, + localizedStrings, +}: TextReadingStatusProps) { + if (index.status === 'error') { + return ( + + ); + } + return ( +

    + + {formatReplacementString(localizedStrings['%interlinearizer_concordance_loading%'], { + read: index.booksRead, + total: index.bookCount, + })} +

    + ); +} diff --git a/src/components/__mocks__/TextReadingStatus.tsx b/src/components/__mocks__/TextReadingStatus.tsx new file mode 100644 index 000000000..1e6f81c7b --- /dev/null +++ b/src/components/__mocks__/TextReadingStatus.tsx @@ -0,0 +1,14 @@ +import type { ConcordanceIndex } from '../../hooks/useConcordanceIndex'; + +export const { TEXT_READING_STRING_KEYS } = jest.requireActual('../TextReadingStatus'); + +/** Stands in for the reading status, exposing the index state it was handed. */ +export default function TextReadingStatus({ + index, +}: Readonly<{ index: Pick }>) { + return ( +

    + {`${index.booksRead} of ${index.bookCount}`} +

    + ); +} diff --git a/src/hooks/useConcordanceIndex.ts b/src/hooks/useConcordanceIndex.ts index e6fff1376..4402635ec 100644 --- a/src/hooks/useConcordanceIndex.ts +++ b/src/hooks/useConcordanceIndex.ts @@ -24,10 +24,19 @@ export interface UseConcordanceIndexArgs { writingSystem: string; /** The book the editor has loaded, which stands in for that book's reading while it is current. */ liveBook: Book | undefined; - /** Whether the index is wanted; nothing is read until it first is. */ + /** Whether the index is wanted; nothing is read until it first is, or is requested. */ enabled: boolean; /** Whether the concordance is on screen; the entries take in live-book edits only while it is. */ shown: boolean; + /** Reads every book again, as a refresh does, whenever this changes. */ + readKey?: unknown; + /** Receives each book as it is read, before only its index is kept. */ + onBookRead?: (book: Book) => void; + /** + * Receives, in canonical order, the books found to have text once every book is read; not called + * for a reading that any book failed. + */ + onTextRead?: (bookIds: readonly string[]) => void; } /** What {@link useConcordanceIndex} hands back. */ @@ -39,8 +48,15 @@ export interface ConcordanceIndex { bookCount: number; /** One entry per form across every book; empty until every book has been read. */ entries: readonly ConcordanceEntry[]; + /** + * Every form the text holds, the live book's as it now reads; `undefined` until every book has + * been read. + */ + textForms: ReadonlySet | undefined; /** Reads every book again, the live one aside. */ refresh: () => void; + /** Wants the index from now on, whatever `enabled` says. */ + request: () => void; } /** @@ -63,20 +79,20 @@ function presentBookIds(booksPresent: string): string[] { */ const READ_CONCURRENCY = 4; -/** Reads a book and indexes it, `undefined` when the project serves no text for it. */ +/** Reads and tokenizes a book, `undefined` when the project serves no text for it. */ async function readBook( usjPdp: Pick, bookId: string, writingSystem: string, -): Promise { +): Promise { const usj = await usjPdp.getBookUSJ({ book: bookId, chapterNum: 1, verseNum: 1 }); if (!usj) return undefined; - return indexBook(tokenizeBook(extractBookFromUsj(usj, writingSystem))); + return tokenizeBook(extractBookFromUsj(usj, writingSystem)); } /** - * Indexes every book of a source project for the concordance, reading them the first time the index - * is wanted and keeping them for as long as the caller stays mounted. + * Indexes every book of a source project, reading them the first time the index is wanted or + * requested and keeping them for as long as the caller stays mounted. * * Every book but the live one is a reading taken when the index was built. The live book replaces * its reading whenever its text changes, and a book the editor moves off keeps the last live @@ -88,6 +104,9 @@ export default function useConcordanceIndex({ liveBook, enabled, shown, + onBookRead, + onTextRead, + readKey, }: UseConcordanceIndexArgs): ConcordanceIndex { const [status, setStatus] = useState('idle'); const [progress, setProgress] = useState({ booksRead: 0, bookCount: 0 }); @@ -98,6 +117,9 @@ export default function useConcordanceIndex({ () => new Map(), ); + const [requested, setRequested] = useState(false); + const wanted = enabled || requested; + /** Bumped to read every book again. */ const [generation, setGeneration] = useState(0); @@ -106,11 +128,17 @@ export default function useConcordanceIndex({ const writingSystemRef = useRef(writingSystem); writingSystemRef.current = writingSystem; + // Read through refs so a read in flight calls whichever callbacks are current when it lands. + const onBookReadRef = useRef(onBookRead); + onBookReadRef.current = onBookRead; + const onTextReadRef = useRef(onTextRead); + onTextReadRef.current = onTextRead; + const readingsRef = useRef(readings); readingsRef.current = readings; useEffect(() => { - if (!enabled) return undefined; + if (!wanted) return undefined; let isCurrent = true; setStatus('loading'); setProgress({ booksRead: 0, bookCount: 0 }); @@ -129,15 +157,17 @@ export default function useConcordanceIndex({ const readNext = async (): Promise => { const bookId = unread.shift(); if (bookId === undefined) return; + let book: Book | undefined; try { - const reading = await readBook(usjPdp, bookId, writingSystemRef.current); - if (reading) read.set(bookId, reading); + book = await readBook(usjPdp, bookId, writingSystemRef.current); + if (book) read.set(bookId, indexBook(book)); else logger.warn(`Concordance: project ${projectId} has no text for ${bookId}`); } catch (e) { booksFailed += 1; logger.warn(`Concordance: skipping ${bookId} in project ${projectId}`, e); } if (!isCurrent) return; + if (book) onBookReadRef.current?.(book); booksRead += 1; setProgress({ booksRead, bookCount: bookIds.length }); await readNext(); @@ -152,6 +182,7 @@ export default function useConcordanceIndex({ } setReadings(read); setStatus('ready'); + if (booksFailed === 0) onTextReadRef.current?.(bookIds.filter((id) => read.has(id))); } catch (e) { if (!isCurrent) return; logger.error(`Concordance: could not list the books of project ${projectId}`, e); @@ -161,12 +192,12 @@ export default function useConcordanceIndex({ return () => { isCurrent = false; }; - }, [enabled, projectId, generation]); + }, [wanted, projectId, generation]); /** The live book, indexed only once the index is wanted. */ const liveIndex = useMemo( - () => (enabled && liveBook ? indexBook(liveBook) : undefined), - [enabled, liveBook], + () => (wanted && liveBook ? indexBook(liveBook) : undefined), + [wanted, liveBook], ); useEffect(() => { @@ -188,6 +219,14 @@ export default function useConcordanceIndex({ setGeneration((g) => g + 1); }, []); + const [readKeyRead, setReadKeyRead] = useState(readKey); + if (readKey !== readKeyRead) { + setReadKeyRead(readKey); + refresh(); + } + + const request = useCallback(() => setRequested(true), []); + const collator = useMemo(() => collatorForTag(writingSystem), [writingSystem]); // Held back while hidden: merging every book again is too costly to repeat for edits nobody sees. @@ -201,8 +240,19 @@ export default function useConcordanceIndex({ return buildConcordanceEntries(books.values(), collator); }, [status, readings, mergedLiveVersions, collator]); + const textForms = useMemo(() => { + if (status !== 'ready' || !readings) return undefined; + const books = new Map(readings); + liveVersions.forEach((index, book) => books.set(book, index)); + const forms = new Set(); + books.forEach(({ occurrencesByForm }) => + occurrencesByForm.forEach((_, form) => forms.add(form)), + ); + return forms; + }, [status, readings, liveVersions]); + return useMemo( - () => ({ status, ...progress, entries, refresh }), - [status, progress, entries, refresh], + () => ({ status, ...progress, entries, textForms, refresh, request }), + [status, progress, entries, textForms, refresh, request], ); } diff --git a/src/utils/analysis-book.ts b/src/utils/analysis-book.ts index 15c6dc384..a43b759bd 100644 --- a/src/utils/analysis-book.ts +++ b/src/utils/analysis-book.ts @@ -11,6 +11,15 @@ export function bookOfRef(ref: string): string { return spaceIndex === -1 ? ref : ref.slice(0, spaceIndex); } +/** Lists the books an analysis has a link of any kind into. */ +export function booksLinkedIn(analysis: TextAnalysis): ReadonlySet { + return new Set([ + ...analysis.tokenAnalysisLinks.map((link) => bookOfRef(link.token.tokenRef)), + ...analysis.segmentAnalysisLinks.map((link) => bookOfRef(link.segmentId)), + ...analysis.phraseAnalysisLinks.map((link) => bookOfRef(link.tokens[0].tokenRef)), + ]); +} + /** * Partition key for payloads no link references, which describe a spelling rather than any one * occurrence and so belong to no book. Its space cannot occur in a book code, so it never collides diff --git a/src/utils/analysis-query.ts b/src/utils/analysis-query.ts index d1dbf9bfe..851e7c218 100644 --- a/src/utils/analysis-query.ts +++ b/src/utils/analysis-query.ts @@ -11,6 +11,7 @@ import type { } from 'interlinearizer'; import type { Collator } from 'platform-bible-utils'; import { bookOfRef } from './analysis-book'; +import { normalizeSurfaceForm } from './analysis-identity'; import { multiStringText } from './multi-string'; import { foldForSearch } from './search-fold'; import { firstVerseNumber } from './verse-ref'; @@ -40,6 +41,8 @@ export interface HeadingPlacement { export interface CatalogRow { analysisId: string; surfaceText: string; + /** `surfaceText` normalized as the text's word forms are, for finding it in the text. */ + form: string; /** * Gloss in the scope's analysis language; `''` when the analysis has none, blank counting as * none. @@ -101,6 +104,8 @@ export interface CatalogFilters { zeroUsages?: boolean; /** Keeps only rows with a stale location, in one of the selected books where `books` is active. */ stale?: boolean; + /** Keeps only rows whose form the query's `textForms` lacks, and none while the text is unread. */ + notInText?: boolean; /** Keeps only rows with no gloss in the scope's analysis language. */ missingGloss?: boolean; /** Keeps rows according to whether they carry a morpheme breakdown. */ @@ -121,6 +126,8 @@ export interface CatalogQuery { surfaceCollator: Collator; /** Collates glosses, so ordering follows the analysis language rather than code points. */ glossCollator: Collator; + /** Every normalized word form the source text holds, `undefined` while it is unread. */ + textForms?: ReadonlySet; } /** @@ -310,6 +317,7 @@ export function buildCatalogRows( return { analysisId: ta.id, surfaceText: ta.surfaceText, + form: normalizeSurfaceForm(ta.surfaceText), gloss: multiStringText(ta.gloss, scope.analysisLanguage), glosses: ta.gloss, glossSenseRef: ta.glossSenseRef, @@ -541,6 +549,7 @@ export function reconcileFilters(filters: CatalogFilters, facets: CatalogFacets) const reconciled: CatalogFilters = { ...(filters.zeroUsages !== undefined && { zeroUsages: filters.zeroUsages }), ...(filters.stale !== undefined && { stale: filters.stale }), + ...(filters.notInText !== undefined && { notInText: filters.notInText }), ...(filters.missingGloss !== undefined && { missingGloss: filters.missingGloss }), ...(filters.morphemes !== undefined && { morphemes: filters.morphemes }), ...(books && { books }), @@ -579,7 +588,11 @@ function passesFeatures(row: CatalogRow, selected: CatalogFilters['features']): } /** Whether the row survives every active filter. */ -function passesFilters(row: CatalogRow, filters: CatalogFilters): boolean { +function passesFilters( + row: CatalogRow, + filters: CatalogFilters, + textForms: ReadonlySet | undefined, +): boolean { const { books } = filters; if (isActive(books) && !books.some((book) => row.books.has(book))) return false; if (!passesValue(filters.pos, row.pos)) return false; @@ -591,6 +604,7 @@ function passesFilters(row: CatalogRow, filters: CatalogFilters): boolean { !row.staleLocations.some((place) => !isActive(books) || books.includes(place.book)) ) return false; + if (filters.notInText && (!textForms || textForms.has(row.form))) return false; if (filters.missingGloss && row.gloss !== '') return false; if (filters.morphemes) { const hasMorphemes = row.morphemes.length > 0; @@ -654,6 +668,9 @@ export function applyCatalogQuery( // it is whitespace only once folded. const search = foldForSearch(collapseLineEndings(query.search)).trim(); return rows - .filter((row) => row.searchText.includes(search) && passesFilters(row, query.filters)) + .filter( + (row) => + row.searchText.includes(search) && passesFilters(row, query.filters, query.textForms), + ) .toSorted((a, b) => compareBySort(a, b, query)); } diff --git a/src/utils/reanchor-draft.ts b/src/utils/reanchor-draft.ts index b1cbab251..1d01979bc 100644 --- a/src/utils/reanchor-draft.ts +++ b/src/utils/reanchor-draft.ts @@ -20,3 +20,14 @@ export function reanchorDraftToBook(verseBook: Book): BookPass { }; }; } + +/** Builds the pass that re-anchors a draft's content to a book the project no longer has. */ +export function reanchorDraftToMissingBook(bookCode: string): BookPass { + return reanchorDraftToBook({ + id: bookCode, + bookRef: bookCode, + textVersion: '', + segments: [], + duplicateVerseIds: [], + }); +} diff --git a/src/utils/stale-free-translations.ts b/src/utils/stale-free-translations.ts index 6d33c05f1..351862f54 100644 --- a/src/utils/stale-free-translations.ts +++ b/src/utils/stale-free-translations.ts @@ -2,7 +2,7 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Book, Segment } from 'interlinearizer'; import { bookOfRef } from './analysis-book'; import { verseOfTokenRef } from './reanchor-analysis'; -import { segmentContainsVerse } from './verse-ref'; +import { firstVerseNumber, segmentContainsVerse } from './verse-ref'; /** A free translation whose segment's text has changed since it was written. */ export type StaleFreeTranslation = Readonly<{ @@ -86,13 +86,47 @@ function vanishedHeadingPlaces( ); } +/** + * Reads where a verse id, or that of a heading filed under the verse, falls among the book's + * verses: its chapter, then the first verse it names. + */ +function versePosition(verse: string): readonly [number, number] { + const [chapterPart, versePart] = verse + .slice(verse.indexOf(' ') + 1) + .split('/')[0] + .split(':'); + /* v8 ignore next -- a verse id always names a verse number */ + return [Number(chapterPart), firstVerseNumber(versePart) ?? 0]; +} + +/** + * Picks the place a translation of a verse the book no longer holds falls back to: the last place + * of the nearest earlier verse, else the book's first place, else `undefined` for a book with + * none. + */ +function placeBefore( + verse: string, + placesByVerse: ReadonlyMap, +): Place | undefined { + const [chapter, verseNumber] = versePosition(verse); + const inOrder = [...placesByVerse] + .flatMap(([placeVerse, places]) => { + const [placeChapter, placeVerseNumber] = versePosition(placeVerse); + const isEarlier = + placeChapter < chapter || (placeChapter === chapter && placeVerseNumber < verseNumber); + return places.map((place) => ({ place, isEarlier })); + }) + .sort((a, b) => a.place.order - b.place.order); + return (inOrder.findLast((entry) => entry.isEarlier) ?? inOrder[0])?.place; +} + /** * Files each of the book's stale free translations under the segment that shows it: the segment it * was written for, or, where that segment has since vanished, the one now covering the position it * began at. A vanished heading's translation goes to its verse's heading of the same marker, else - * the start of its verse. Each segment's list runs in document order of those positions. - * - * A translation whose verse the book no longer holds at all is shown nowhere. + * the start of its verse. A translation whose verse the book no longer holds goes to the segment + * ending the nearest earlier verse, else to the book's first segment. Each segment's list runs in + * document order of those positions. */ export function placeStaleFreeTranslations( stale: readonly StaleFreeTranslation[], @@ -103,9 +137,10 @@ export function placeStaleFreeTranslations( if (bookOfRef(translation.segmentId) !== book.bookRef) return []; const { verse, offset } = positionOf(translation.segmentId); const places = placesByVerse.get(verse) ?? vanishedHeadingPlaces(verse, placesByVerse); - if (!places) return []; - const covering = places.findLast((place) => place.offset <= offset) ?? places[0]; - return [{ translation, covering }]; + const covering = places + ? (places.findLast((place) => place.offset <= offset) ?? places[0]) + : placeBefore(verse, placesByVerse); + return covering ? [{ translation, covering }] : []; }); const bySegment = new Map(); From fbec5946e3a6c63515038508f94a332000e00035 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 7 Oct 2026 12:31:51 -0600 Subject: [PATCH 02/10] Don't mark forms absent when a book fails to read --- .../components/AnalysisCatalogPanel.test.tsx | 1 + .../ConcordanceIndexContext.test.tsx | 1 + .../components/ConcordancePanel.test.tsx | 1 + .../components/InterlinearizerLoader.test.tsx | 3 ++ .../components/TextReadingStatus.test.tsx | 17 ++++++- .../hooks/useConcordanceIndex.test.ts | 48 +++++++++++++++++++ src/components/TextReadingStatus.tsx | 4 +- src/hooks/useConcordanceIndex.ts | 15 ++++-- 8 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 4a74b7f4c..f910644a8 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -95,6 +95,7 @@ function makeIndex(overrides: Partial = {}): ConcordanceIndex booksRead: 0, bookCount: 0, entries: [], + isPartial: false, textForms: undefined, refresh: () => {}, request: () => {}, diff --git a/src/__tests__/components/ConcordanceIndexContext.test.tsx b/src/__tests__/components/ConcordanceIndexContext.test.tsx index c36782315..7e2d04823 100644 --- a/src/__tests__/components/ConcordanceIndexContext.test.tsx +++ b/src/__tests__/components/ConcordanceIndexContext.test.tsx @@ -17,6 +17,7 @@ const INDEX: ConcordanceIndex = { booksRead: 1, bookCount: 1, entries: [], + isPartial: false, textForms: undefined, refresh: () => {}, request: () => {}, diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index e24b5ed68..1881832b5 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -66,6 +66,7 @@ function makeIndex(overrides: Partial = {}): ConcordanceIndex booksRead: 2, bookCount: 2, entries: ENTRIES, + isPartial: false, textForms: undefined, refresh: () => {}, request: () => {}, diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 46d3be291..ff54a9435 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -604,6 +604,7 @@ function mockUnreadConcordanceIndex(): void { booksRead: 0, bookCount: 0, entries: [], + isPartial: false, textForms: undefined, refresh: jest.fn(), request: jest.fn(), @@ -4027,6 +4028,7 @@ describe('InterlinearizerLoader', () => { booksRead: 0, bookCount: 0, entries: [], + isPartial: false, textForms: undefined, refresh: jest.fn(), request: jest.fn(), @@ -4160,6 +4162,7 @@ describe('InterlinearizerLoader', () => { booksRead: 0, bookCount: 0, entries: [], + isPartial: false, textForms: undefined, refresh: jest.fn(), request: jest.fn(), diff --git a/src/__tests__/components/TextReadingStatus.test.tsx b/src/__tests__/components/TextReadingStatus.test.tsx index 2d1cd19ae..21b4d2db5 100644 --- a/src/__tests__/components/TextReadingStatus.test.tsx +++ b/src/__tests__/components/TextReadingStatus.test.tsx @@ -14,7 +14,7 @@ describe('TextReadingStatus', () => { render( , ); @@ -27,7 +27,20 @@ describe('TextReadingStatus', () => { render( , + ); + + expect(screen.getByTestId('panel-error')).toHaveTextContent('The text could not be read.'); + expect(screen.queryByTestId('panel-loading')).not.toBeInTheDocument(); + }); + + it('says the text could not be read when a book of it failed to read', () => { + render( + , ); diff --git a/src/__tests__/hooks/useConcordanceIndex.test.ts b/src/__tests__/hooks/useConcordanceIndex.test.ts index ba0867de7..6c1272f9e 100644 --- a/src/__tests__/hooks/useConcordanceIndex.test.ts +++ b/src/__tests__/hooks/useConcordanceIndex.test.ts @@ -198,6 +198,54 @@ describe('useConcordanceIndex', () => { expect(onTextRead).not.toHaveBeenCalled(); }); + it('withholds the text forms while a book of it failed to read', async () => { + serveProject('11', async (id) => { + if (id === 'GEN') throw new Error('unreadable'); + return usj(id, 'v1'); + }); + + const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + + await waitFor(() => expect(result.current.status).toBe('ready')); + expect(result.current.isPartial).toBe(true); + expect(result.current.textForms).toBeUndefined(); + }); + + it('drops a partial reading while the text is read again', async () => { + const secondGenesis = deferred(); + let genesisReads = 0; + serveProject('11', (id) => { + if (id !== 'GEN') return Promise.resolve(usj(id, 'v1')); + genesisReads += 1; + return genesisReads === 1 ? Promise.reject(new Error('unreadable')) : secondGenesis.promise; + }); + const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + await waitFor(() => expect(result.current.isPartial).toBe(true)); + + act(() => result.current.refresh()); + + expect(result.current.status).toBe('loading'); + expect(result.current.isPartial).toBe(false); + }); + + it('reports the text forms once a re-read takes in the book that failed', async () => { + let genesisReads = 0; + serveProject('11', async (id) => { + if (id !== 'GEN') return usj(id, 'v1'); + genesisReads += 1; + if (genesisReads === 1) throw new Error('unreadable'); + return usj(id, 'v1'); + }); + const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + await waitFor(() => expect(result.current.isPartial).toBe(true)); + + act(() => result.current.refresh()); + + await waitFor(() => expect(result.current.textForms).toBeDefined()); + expect([...(result.current.textForms ?? [])].sort()).toEqual(['EXO@v1', 'GEN@v1']); + expect(result.current.isPartial).toBe(false); + }); + it('reports an error when no book could be read', async () => { serveProject('11', async (id) => { if (id === 'GEN') throw new Error('unreadable'); diff --git a/src/components/TextReadingStatus.tsx b/src/components/TextReadingStatus.tsx index e72d1a6ec..4c74ee18c 100644 --- a/src/components/TextReadingStatus.tsx +++ b/src/components/TextReadingStatus.tsx @@ -10,7 +10,7 @@ export const TEXT_READING_STRING_KEYS = [ /** Props for {@link TextReadingStatus}. */ type TextReadingStatusProps = Readonly<{ - index: Pick; + index: Pick; /** Prefixes the `-loading` and `-error` test ids the status is found by. */ idPrefix: string; /** Resolved localizations covering at least {@link TEXT_READING_STRING_KEYS}. */ @@ -23,7 +23,7 @@ export default function TextReadingStatus({ idPrefix, localizedStrings, }: TextReadingStatusProps) { - if (index.status === 'error') { + if (index.status === 'error' || index.isPartial) { return ( | undefined; /** Reads every book again, the live one aside. */ @@ -111,6 +113,7 @@ export default function useConcordanceIndex({ const [status, setStatus] = useState('idle'); const [progress, setProgress] = useState({ booksRead: 0, bookCount: 0 }); const [readings, setReadings] = useState>(); + const [isPartial, setIsPartial] = useState(false); /** Live versions of books the editor has had loaded, each newer than the book's reading. */ const [liveVersions, setLiveVersions] = useState>( @@ -141,6 +144,7 @@ export default function useConcordanceIndex({ if (!wanted) return undefined; let isCurrent = true; setStatus('loading'); + setIsPartial(false); setProgress({ booksRead: 0, bookCount: 0 }); (async () => { try { @@ -181,6 +185,7 @@ export default function useConcordanceIndex({ return; } setReadings(read); + setIsPartial(booksFailed > 0); setStatus('ready'); if (booksFailed === 0) onTextReadRef.current?.(bookIds.filter((id) => read.has(id))); } catch (e) { @@ -241,7 +246,7 @@ export default function useConcordanceIndex({ }, [status, readings, mergedLiveVersions, collator]); const textForms = useMemo(() => { - if (status !== 'ready' || !readings) return undefined; + if (status !== 'ready' || !readings || isPartial) return undefined; const books = new Map(readings); liveVersions.forEach((index, book) => books.set(book, index)); const forms = new Set(); @@ -249,10 +254,10 @@ export default function useConcordanceIndex({ occurrencesByForm.forEach((_, form) => forms.add(form)), ); return forms; - }, [status, readings, liveVersions]); + }, [status, readings, isPartial, liveVersions]); return useMemo( - () => ({ status, ...progress, entries, textForms, refresh, request }), - [status, progress, entries, textForms, refresh, request], + () => ({ status, ...progress, entries, isPartial, textForms, refresh, request }), + [status, progress, entries, isPartial, textForms, refresh, request], ); } From a3148efbc2e258bcb55c1cb96f60d3ada555e034 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 7 Oct 2026 12:55:37 -0600 Subject: [PATCH 03/10] Pass staleCoversDraft in the catalog's analysis-language tests --- src/__tests__/components/AnalysisCatalogPanel.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index f910644a8..aa90389eb 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -1454,6 +1454,7 @@ describe('AnalysisCatalogPanel', () => { onShowConcordance={() => {}} showMorphology sourceLanguageTag="el" + staleCoversDraft={false} /> ); From 2c4a55f97449c104fc53aaab06196f122c18170c Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 7 Oct 2026 14:03:35 -0600 Subject: [PATCH 04/10] Note a partial reading above the concordance list --- contributions/localizedStrings.json | 1 + .../components/ConcordancePanel.test.tsx | 15 +++++++++++++++ src/components/ConcordancePanel.tsx | 10 ++++++++++ 3 files changed, 26 insertions(+) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 8ee5cd464..b88eee19e 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -48,6 +48,7 @@ "%interlinearizer_concordance_loading%": "Reading books… {read} of {total}", "%interlinearizer_concordance_error%": "The text of this project could not be read.", "%interlinearizer_concordance_empty%": "The source text has no words to list.", + "%interlinearizer_concordance_partial%": "Some books could not be read, so their words are missing from this list.", "%interlinearizer_concordance_occurrenceCount%": "Occurrences in the whole text", "%interlinearizer_concordance_occurrenceCountInBook%": "Occurrences in {book}", "%interlinearizer_concordance_status_analyzed%": "Analyzed", diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index 1881832b5..ef7b595f4 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -161,6 +161,21 @@ describe('ConcordancePanel', () => { ); }); + it('lists what was read beside a notice when a book failed to read', () => { + renderPanel({ index: makeIndex({ isPartial: true }) }); + + expect(screen.getByTestId('concordance-partial')).toHaveTextContent( + '%interlinearizer_concordance_partial%', + ); + expect(screen.getAllByTestId('concordance-row')).not.toHaveLength(0); + }); + + it('shows no partial-reading notice when every book was read', () => { + renderPanel(); + + expect(screen.queryByTestId('concordance-partial')).not.toBeInTheDocument(); + }); + it('lists every form of the text, most frequent first', () => { renderPanel(); diff --git a/src/components/ConcordancePanel.tsx b/src/components/ConcordancePanel.tsx index 74dc43ae6..d54f30214 100644 --- a/src/components/ConcordancePanel.tsx +++ b/src/components/ConcordancePanel.tsx @@ -33,6 +33,7 @@ const STRING_KEYS = [ '%interlinearizer_concordance_close%', '%interlinearizer_concordance_refresh%', '%interlinearizer_concordance_empty%', + '%interlinearizer_concordance_partial%', '%interlinearizer_concordance_occurrenceCountInBook%', ...SIDE_PANEL_TAB_STRING_KEYS, ...CONCORDANCE_ROW_STRING_KEYS, @@ -222,6 +223,15 @@ export default function ConcordancePanel({ + {index.isPartial && ( +

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

    + )} {body} From 1f229cef53f5698c5d7f24ea110ad743f75ef056 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 7 Oct 2026 14:03:35 -0600 Subject: [PATCH 05/10] Drop a concordance read once its read key moves on --- src/hooks/useConcordanceIndex.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/hooks/useConcordanceIndex.ts b/src/hooks/useConcordanceIndex.ts index 19ba3e28f..1efa320ee 100644 --- a/src/hooks/useConcordanceIndex.ts +++ b/src/hooks/useConcordanceIndex.ts @@ -28,7 +28,10 @@ export interface UseConcordanceIndexArgs { enabled: boolean; /** Whether the concordance is on screen; the entries take in live-book edits only while it is. */ shown: boolean; - /** Reads every book again, as a refresh does, whenever this changes. */ + /** + * Reads every book again, as a refresh does, whenever this changes; a reading begun under an + * earlier value hands nothing on. + */ readKey?: unknown; /** Receives each book as it is read, before only its index is kept. */ onBookRead?: (book: Book) => void; @@ -136,13 +139,18 @@ export default function useConcordanceIndex({ onBookReadRef.current = onBookRead; const onTextReadRef = useRef(onTextRead); onTextReadRef.current = onTextRead; + const readKeyRef = useRef(readKey); + readKeyRef.current = readKey; const readingsRef = useRef(readings); readingsRef.current = readings; useEffect(() => { if (!wanted) return undefined; - let isCurrent = true; + let isAbandoned = false; + const key = readKeyRef.current; + // The key moves on as soon as it renders, ahead of the cleanup that abandons this read. + const isCurrent = () => !isAbandoned && readKeyRef.current === key; setStatus('loading'); setIsPartial(false); setProgress({ booksRead: 0, bookCount: 0 }); @@ -151,7 +159,7 @@ export default function useConcordanceIndex({ const basePdp = await papi.projectDataProviders.get('platform.base', projectId); const bookIds = presentBookIds(await basePdp.getSetting('platformScripture.booksPresent')); const usjPdp = await papi.projectDataProviders.get('platformScripture.USJ_Book', projectId); - if (!isCurrent) return; + if (!isCurrent()) return; setProgress({ booksRead: 0, bookCount: bookIds.length }); const read = new Map(); @@ -170,14 +178,14 @@ export default function useConcordanceIndex({ booksFailed += 1; logger.warn(`Concordance: skipping ${bookId} in project ${projectId}`, e); } - if (!isCurrent) return; + if (!isCurrent()) return; if (book) onBookReadRef.current?.(book); booksRead += 1; setProgress({ booksRead, bookCount: bookIds.length }); await readNext(); }; await Promise.all(Array.from({ length: READ_CONCURRENCY }, readNext)); - if (!isCurrent) return; + if (!isCurrent()) return; // With nothing read, a failure could be hiding text, so an empty list would misreport it. if (read.size === 0 && booksFailed > 0) { logger.error(`Concordance: no book of project ${projectId} could be read`); @@ -189,13 +197,13 @@ export default function useConcordanceIndex({ setStatus('ready'); if (booksFailed === 0) onTextReadRef.current?.(bookIds.filter((id) => read.has(id))); } catch (e) { - if (!isCurrent) return; + if (!isCurrent()) return; logger.error(`Concordance: could not list the books of project ${projectId}`, e); setStatus('error'); } })(); return () => { - isCurrent = false; + isAbandoned = true; }; }, [wanted, projectId, generation]); From c241c8fa42afc467a6413fbf104d63006c739398 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 8 Oct 2026 15:49:22 -0600 Subject: [PATCH 06/10] Split source-text reading out of the concordance index --- contributions/localizedStrings.json | 4 +- docs/reanchoring.md | 7 +- .../components/AnalysisCatalogPanel.test.tsx | 47 ++-- .../ConcordanceIndexContext.test.tsx | 63 ----- .../components/ConcordancePanel.test.tsx | 59 ++--- .../components/InterlinearizerLoader.test.tsx | 219 ++++++------------ .../components/SourceTextContext.test.tsx | 92 ++++++++ .../components/TextReadingStatus.test.tsx | 10 +- .../hooks/useConcordanceEntries.test.ts | 105 +++++++++ ...ex.test.ts => useSourceTextReader.test.ts} | 207 ++++++++--------- .../hooks/useWholeTextReanchor.test.ts | 183 +++++++++++++++ src/__tests__/utils/verse-ref.test.ts | 12 +- src/components/AnalysisCatalogPanel.tsx | 17 +- src/components/CatalogFilterPopover.tsx | 14 +- src/components/ConcordanceIndexContext.tsx | 30 --- src/components/ConcordancePanel.tsx | 23 +- src/components/InterlinearizerLoader.tsx | 93 +++----- src/components/SourceTextContext.tsx | 63 +++++ src/components/TextReadingStatus.tsx | 20 +- .../__mocks__/TextReadingStatus.tsx | 12 +- src/hooks/useConcordanceEntries.ts | 37 +++ ...ordanceIndex.ts => useSourceTextReader.ts} | 129 +++++------ src/hooks/useWholeTextReanchor.ts | 86 +++++++ src/utils/analysis-query.ts | 24 +- src/utils/stale-free-translations.ts | 46 ++-- src/utils/verse-ref.ts | 12 +- 26 files changed, 975 insertions(+), 639 deletions(-) delete mode 100644 src/__tests__/components/ConcordanceIndexContext.test.tsx create mode 100644 src/__tests__/components/SourceTextContext.test.tsx create mode 100644 src/__tests__/hooks/useConcordanceEntries.test.ts rename src/__tests__/hooks/{useConcordanceIndex.test.ts => useSourceTextReader.test.ts} (68%) create mode 100644 src/__tests__/hooks/useWholeTextReanchor.test.ts delete mode 100644 src/components/ConcordanceIndexContext.tsx create mode 100644 src/components/SourceTextContext.tsx create mode 100644 src/hooks/useConcordanceEntries.ts rename src/hooks/{useConcordanceIndex.ts => useSourceTextReader.ts} (65%) create mode 100644 src/hooks/useWholeTextReanchor.ts diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index b88eee19e..1f0e0d1f2 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -45,8 +45,8 @@ "%interlinearizer_concordance_title%": "Concordance", "%interlinearizer_concordance_close%": "Close concordance", "%interlinearizer_concordance_refresh%": "Read the source text again", - "%interlinearizer_concordance_loading%": "Reading books… {read} of {total}", - "%interlinearizer_concordance_error%": "The text of this project could not be read.", + "%interlinearizer_textReading_loading%": "Reading books… {read} of {total}", + "%interlinearizer_textReading_error%": "The text of this project could not be read.", "%interlinearizer_concordance_empty%": "The source text has no words to list.", "%interlinearizer_concordance_partial%": "Some books could not be read, so their words are missing from this list.", "%interlinearizer_concordance_occurrenceCount%": "Occurrences in the whole text", diff --git a/docs/reanchoring.md b/docs/reanchoring.md index 1f383136d..b2e3255b1 100644 --- a/docs/reanchoring.md +++ b/docs/reanchoring.md @@ -21,7 +21,12 @@ Do not grow this into a general diff, fuzzy matching, or cross-verse tracking. F ## Algorithm -When it runs: whenever the loaded book's text, its boundaries, or the draft changes, and for every other book the draft has records in each time the whole source text is read (for the concordance, or a catalog filter that needs the text), for an editable project only. A book the project no longer has is treated as holding no text, so every approval in it goes stale. An imported, read-only project is a record of what was imported and is never healed. A pass that moves nothing leaves the analysis identical, so opening a book neither dirties the draft nor writes storage. +When it runs, for an editable project only: + +- for the loaded book, whenever its text, its boundaries, or the draft changes; +- for every other book the draft has records in, each time the whole source text is read (for the concordance, or a catalog filter that needs the text). + +A book the project no longer has is treated as holding no text, so every approval in it goes stale. An imported, read-only project is a record of what was imported and is never healed. A pass that moves nothing leaves the analysis identical, so opening a book neither dirties the draft nor writes storage. Token and phrase links carry a `TokenSnapshot` (`tokenRef` plus the `surfaceText` it was written against). Approved, candidate, and stale links are the evidence the alignment weighs; rejected and suggested links align afterward, among themselves, against the tokens those leave unclaimed, so a verdict never decides which occurrence an approval names. For each verse independently: diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index aa90389eb..90fe3fdf8 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -11,9 +11,9 @@ import AnalysisCatalogPanel, { type AnalysisCatalogPanelHandle, } from '../../components/AnalysisCatalogPanel'; import { AnalysisStoreProvider, useGlossDispatch } from '../../components/AnalysisStore'; -import { ConcordanceIndexContext } from '../../components/ConcordanceIndexContext'; +import { SourceTextContext } from '../../components/SourceTextContext'; import { InterlinearNavProvider, useInterlinearNav } from '../../components/InterlinearNavContext'; -import type { ConcordanceIndex } from '../../hooks/useConcordanceIndex'; +import type { SourceText } from '../../hooks/useSourceTextReader'; import type { StepSummary } from '../../hooks/useDraftProject'; import { emptyAnalysis } from '../../types/empty-factories'; import type { HeadingPlacement } from '../../utils/analysis-query'; @@ -88,13 +88,14 @@ function FocusPublishProbe({ tokenRef }: Readonly<{ tokenRef: string | undefined return undefined; } -/** Builds an index of the source text as its hook hands it over, unread by default. */ -function makeIndex(overrides: Partial = {}): ConcordanceIndex { +/** Builds the source text as its hook hands it over, unread by default. */ +function makeText(overrides: Partial = {}): SourceText { return { status: 'idle', booksRead: 0, bookCount: 0, - entries: [], + readings: undefined, + liveVersions: new Map(), isPartial: false, textForms: undefined, refresh: () => {}, @@ -131,8 +132,8 @@ type PanelOptions = Partial<{ liveSurfaceText: (tokenRef: string) => string | undefined; /** The word the view beside the panel has focused, or none. */ focusedTokenRef: string; - /** The index of the source text. Defaults to one not yet read. */ - index: ConcordanceIndex; + /** The source text. Defaults to one not yet read. */ + text: SourceText; /** Whether every book has been checked for stale places. Defaults to only the opened ones. */ staleCoversDraft: boolean; }>; @@ -167,9 +168,9 @@ function PanelProviders({ > - + {children} - + ); @@ -2033,7 +2034,7 @@ describe('AnalysisCatalogPanel', () => { it('asks for the text to be read when filtering for them', async () => { const request = jest.fn(); - renderPanel({ analysis: ONLY_STALE, index: makeIndex({ request }) }); + renderPanel({ analysis: ONLY_STALE, text: makeText({ request }) }); await openFilters(); await userEvent.click( @@ -2212,7 +2213,7 @@ describe('AnalysisCatalogPanel', () => { it('asks for the text to be read when filtering for them', async () => { const request = jest.fn(); - renderPanel({ analysis, index: makeIndex({ request }) }); + renderPanel({ analysis, text: makeText({ request }) }); await openFilters(); await userEvent.click(notInTextFilter()); @@ -2220,9 +2221,19 @@ describe('AnalysisCatalogPanel', () => { expect(request).toHaveBeenCalled(); }); + it('asks for the text to be read when narrowed to them from outside', () => { + const request = jest.fn(); + const ref = createRef(); + renderPanel({ ref, analysis, text: makeText({ request }) }); + + act(() => ref.current?.filterTo({ notInText: true })); + + expect(request).toHaveBeenCalled(); + }); + it('keeps only the analyses whose form the text lacks once it is read', async () => { - const index = makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }); - renderPanel({ analysis, index }); + const text = makeText({ status: 'ready', textForms: new Set(['λόγος']) }); + renderPanel({ analysis, text }); await openFilters(); await userEvent.click(notInTextFilter()); @@ -2231,8 +2242,8 @@ describe('AnalysisCatalogPanel', () => { }); it('shows how far reading the text has got while filtering for them', async () => { - const index = makeIndex({ status: 'loading', booksRead: 12, bookCount: 66 }); - renderPanel({ analysis, index }); + const text = makeText({ status: 'loading', booksRead: 12, bookCount: 66 }); + renderPanel({ analysis, text }); await openFilters(); await userEvent.click(notInTextFilter()); @@ -2242,7 +2253,7 @@ describe('AnalysisCatalogPanel', () => { }); it('says so when the text could not be read', async () => { - renderPanel({ analysis, index: makeIndex({ status: 'error' }) }); + renderPanel({ analysis, text: makeText({ status: 'error' }) }); await openFilters(); await userEvent.click(notInTextFilter()); @@ -2253,7 +2264,7 @@ describe('AnalysisCatalogPanel', () => { it('marks an analysis whose form the text lacks once it is read', () => { renderPanel({ analysis, - index: makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }), + text: makeText({ status: 'ready', textForms: new Set(['λόγος']) }), }); expect(within(rowFor('gone')).getByTestId('catalog-row-not-in-text')).toBeInTheDocument(); @@ -2262,7 +2273,7 @@ describe('AnalysisCatalogPanel', () => { it('does not mark an analysis whose form the text holds', () => { renderPanel({ analysis, - index: makeIndex({ status: 'ready', textForms: new Set(['λόγος']) }), + text: makeText({ status: 'ready', textForms: new Set(['λόγος']) }), }); expect( diff --git a/src/__tests__/components/ConcordanceIndexContext.test.tsx b/src/__tests__/components/ConcordanceIndexContext.test.tsx deleted file mode 100644 index 7e2d04823..000000000 --- a/src/__tests__/components/ConcordanceIndexContext.test.tsx +++ /dev/null @@ -1,63 +0,0 @@ -/// - -import { renderHook } from '@testing-library/react'; -import type { ReactNode } from 'react'; -import { - ConcordanceIndexProvider, - useConcordanceIndexContext, -} from '../../components/ConcordanceIndexContext'; -import useConcordanceIndex, { type ConcordanceIndex } from '../../hooks/useConcordanceIndex'; -import { GEN_1_1_BOOK } from '../test-helpers'; - -// Reading books is the hook's own concern; the provider only hands on whatever it builds. -jest.mock('../../hooks/useConcordanceIndex'); - -const INDEX: ConcordanceIndex = { - status: 'ready', - booksRead: 1, - bookCount: 1, - entries: [], - isPartial: false, - textForms: undefined, - refresh: () => {}, - request: () => {}, -}; - -describe('ConcordanceIndexProvider', () => { - it('builds the index from its props and hands it to what it wraps', () => { - jest.mocked(useConcordanceIndex).mockReturnValue(INDEX); - const wrapper = ({ children }: Readonly<{ children: ReactNode }>) => ( - - {children} - - ); - - const { result } = renderHook(() => useConcordanceIndexContext(), { wrapper }); - - expect(result.current).toBe(INDEX); - expect(useConcordanceIndex).toHaveBeenCalledWith({ - enabled: true, - liveBook: GEN_1_1_BOOK, - projectId: 'src', - shown: true, - writingSystem: 'en', - }); - }); -}); - -describe('useConcordanceIndexContext', () => { - it('throws outside a provider', () => { - // React logs the render error before rethrowing it; silenced so the expected throw is quiet. - jest.spyOn(console, 'error').mockImplementation(() => {}); - - expect(() => renderHook(() => useConcordanceIndexContext())).toThrow( - 'useConcordanceIndexContext must be used within its provider', - ); - }); -}); diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index ef7b595f4..9f3ae5b4d 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -8,12 +8,12 @@ import type { TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; import { Collator } from 'platform-bible-utils'; import { useEffect, type ReactNode } from 'react'; import { AnalysisStoreProvider, useGlossDispatch } from '../../components/AnalysisStore'; -import { ConcordanceIndexContext } from '../../components/ConcordanceIndexContext'; import ConcordancePanel from '../../components/ConcordancePanel'; import { InterlinearNavProvider, useInterlinearNav } from '../../components/InterlinearNavContext'; -import type { ConcordanceIndex } from '../../hooks/useConcordanceIndex'; +import { ConcordanceEntriesContext, SourceTextContext } from '../../components/SourceTextContext'; +import type { SourceText } from '../../hooks/useSourceTextReader'; import { emptyAnalysis } from '../../types/empty-factories'; -import { buildConcordanceEntries, indexBook } from '../../utils/concordance'; +import { buildConcordanceEntries, indexBook, type ConcordanceEntry } from '../../utils/concordance'; import { defaultScrRef, FIXTURE_STAMPS, makeScrollGroupHook, makeVerseBook } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; @@ -59,13 +59,14 @@ const ENTRIES = buildConcordanceEntries( new Collator('en'), ); -/** Builds an index as the loading hook hands it over, ready with {@link ENTRIES} by default. */ -function makeIndex(overrides: Partial = {}): ConcordanceIndex { +/** Builds the source text as the reading hook hands it over, ready by default. */ +function makeText(overrides: Partial = {}): SourceText { return { status: 'ready', booksRead: 2, bookCount: 2, - entries: ENTRIES, + readings: new Map(), + liveVersions: new Map(), isPartial: false, textForms: undefined, refresh: () => {}, @@ -85,7 +86,9 @@ function link(analysisId: string, tokenRef: string): TokenAnalysisLink { } type PanelOptions = Partial<{ - index: ConcordanceIndex; + text: SourceText; + /** The entries listed, {@link ENTRIES} by default. */ + entries: readonly ConcordanceEntry[]; analysis: TextAnalysis; currentBook: string; onClose: () => void; @@ -107,14 +110,16 @@ function panelTree(options: PanelOptions = {}) { > {options.beside} - - {})} - onShowCatalog={options.onShowCatalog ?? (() => {})} - sourceLanguageTag="en" - /> - + + + {})} + onShowCatalog={options.onShowCatalog ?? (() => {})} + sourceLanguageTag="en" + /> + + ); @@ -133,7 +138,7 @@ function rowFor(form: string): HTMLElement { beforeEach(() => { mockKeyAsValueLocalizedStrings({ - '%interlinearizer_concordance_loading%': 'Reading {read} of {total}', + '%interlinearizer_textReading_loading%': 'Reading {read} of {total}', '%interlinearizer_concordance_occurrenceCountInBook%': 'in {book}', }); claimedFocusRequest = undefined; @@ -141,20 +146,20 @@ beforeEach(() => { describe('ConcordancePanel', () => { it('shows how far reading the books has got while the index is built', () => { - renderPanel({ index: makeIndex({ status: 'loading', booksRead: 12, bookCount: 66 }) }); + renderPanel({ text: makeText({ status: 'loading', booksRead: 12, bookCount: 66 }) }); expect(screen.getByTestId('text-reading-status')).toHaveTextContent('12 of 66'); expect(screen.queryByTestId('concordance-row')).not.toBeInTheDocument(); }); it('says so when the books could not be listed', () => { - renderPanel({ index: makeIndex({ status: 'error' }) }); + renderPanel({ text: makeText({ status: 'error' }) }); expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-status', 'error'); }); it('says so when the text has no words', () => { - renderPanel({ index: makeIndex({ entries: [] }) }); + renderPanel({ entries: [] }); expect(screen.getByTestId('concordance-empty')).toHaveTextContent( '%interlinearizer_concordance_empty%', @@ -162,7 +167,7 @@ describe('ConcordancePanel', () => { }); it('lists what was read beside a notice when a book failed to read', () => { - renderPanel({ index: makeIndex({ isPartial: true }) }); + renderPanel({ text: makeText({ isPartial: true }) }); expect(screen.getByTestId('concordance-partial')).toHaveTextContent( '%interlinearizer_concordance_partial%', @@ -274,7 +279,7 @@ describe('ConcordancePanel', () => { it('reads the text again on refresh', async () => { const refresh = jest.fn(); - renderPanel({ index: makeIndex({ refresh }) }); + renderPanel({ text: makeText({ refresh }) }); await userEvent.click(screen.getByTestId('concordance-refresh')); @@ -294,7 +299,7 @@ describe('ConcordancePanel', () => { }); it('offers no refresh while the text is being read', () => { - renderPanel({ index: makeIndex({ status: 'loading' }) }); + renderPanel({ text: makeText({ status: 'loading' }) }); expect(screen.getByTestId('concordance-refresh')).toBeDisabled(); }); @@ -333,7 +338,7 @@ describe('ConcordancePanel', () => { ); it('mounts only part of it', () => { - renderPanel({ index: makeIndex({ entries: MANY }) }); + renderPanel({ entries: MANY }); const mounted = screen.getAllByTestId('concordance-row').length; expect(mounted).toBeGreaterThan(0); @@ -341,7 +346,7 @@ describe('ConcordancePanel', () => { }); it('mounts more as the end of what is mounted comes into view', () => { - renderPanel({ index: makeIndex({ entries: MANY }) }); + renderPanel({ entries: MANY }); const before = screen.getAllByTestId('concordance-row').length; act(() => { @@ -352,14 +357,14 @@ describe('ConcordancePanel', () => { }); it('starts over at its first rows once a refresh has read the text again', () => { - const { rerender } = renderPanel({ index: makeIndex({ entries: MANY }) }); + const { rerender } = renderPanel({ entries: MANY }); const initial = screen.getAllByTestId('concordance-row').length; act(() => { global.triggerIntersection(screen.getByTestId('concordance-rows-sentinel'), true); }); - rerender(panelTree({ index: makeIndex({ status: 'loading', entries: [] }) })); - rerender(panelTree({ index: makeIndex({ entries: MANY }) })); + rerender(panelTree({ text: makeText({ status: 'loading' }), entries: [] })); + rerender(panelTree({ entries: MANY })); expect(screen.getAllByTestId('concordance-row')).toHaveLength(initial); }); diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index ff54a9435..52297e1b0 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -24,10 +24,11 @@ import InterlinearizerLoader, { UNDO_NOTIFICATION_DURATION_MS, } from '../../components/InterlinearizerLoader'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; -import useConcordanceIndex, { - type ConcordanceIndex, - type UseConcordanceIndexArgs, -} from '../../hooks/useConcordanceIndex'; +import useConcordanceEntries from '../../hooks/useConcordanceEntries'; +import useSourceTextReader, { + type SourceText, + type UseSourceTextReaderArgs, +} from '../../hooks/useSourceTextReader'; import useInterlinearizerBookData from '../../hooks/useInterlinearizerBookData'; import useLexiconRegistry from '../../hooks/useLexiconRegistry'; import useLostBoundaryDismissal from '../../hooks/useLostBoundaryDismissal'; @@ -55,7 +56,8 @@ import { } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; -jest.mock('../../hooks/useConcordanceIndex'); +jest.mock('../../hooks/useConcordanceEntries'); +jest.mock('../../hooks/useSourceTextReader'); jest.mock('../../hooks/useInterlinearizerBookData'); jest.mock('../../hooks/useLexiconRegistry'); jest.mock('../../hooks/useLostBoundaryDismissal'); @@ -597,18 +599,26 @@ function renderLoader( return { ...result, updateWebViewDefinition }; } -/** Has the index of the source text report itself unread, as it is until something asks for it. */ -function mockUnreadConcordanceIndex(): void { - jest.mocked(useConcordanceIndex).mockReturnValue({ +/** Builds the source text as its hook hands it over, unread by default. */ +function makeSourceText(overrides: Partial = {}): SourceText { + return { status: 'idle', booksRead: 0, bookCount: 0, - entries: [], + readings: undefined, + liveVersions: new Map(), isPartial: false, textForms: undefined, refresh: jest.fn(), request: jest.fn(), - }); + ...overrides, + }; +} + +/** Has the source text report itself unread, as it is until something asks for it. */ +function mockUnreadSourceText(): void { + jest.mocked(useSourceTextReader).mockReturnValue(makeSourceText()); + jest.mocked(useConcordanceEntries).mockReturnValue([]); } /** Configures useInterlinearizerBookData to return the given state. */ @@ -791,7 +801,7 @@ describe('InterlinearizerLoader', () => { }); mockSettings(); mockSourceShortName('WEB'); - mockUnreadConcordanceIndex(); + mockUnreadSourceText(); }); it('shows nav controls when interface mode is power', async () => { @@ -4023,16 +4033,7 @@ describe('InterlinearizerLoader', () => { describe('concordance command', () => { beforeEach(() => { - jest.mocked(useConcordanceIndex).mockReturnValue({ - status: 'ready', - booksRead: 0, - bookCount: 0, - entries: [], - isPartial: false, - textForms: undefined, - refresh: jest.fn(), - request: jest.fn(), - }); + jest.mocked(useSourceTextReader).mockReturnValue(makeSourceText({ status: 'ready' })); }); it('opens the concordance beside the interlinear view', async () => { @@ -4131,12 +4132,12 @@ describe('InterlinearizerLoader', () => { }); await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); - expect(useConcordanceIndex).toHaveBeenLastCalledWith( + expect(useSourceTextReader).toHaveBeenLastCalledWith( expect.objectContaining({ enabled: false }), ); }); - it('indexes the loaded book of this project once the concordance is shown', async () => { + it('reads the text of this project, the loaded book live, once the concordance is shown', async () => { mockBookData({ writingSystem: 'hbo' }); await act(async () => { renderLoader(); @@ -4144,36 +4145,39 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('tab-toolbar-concordance')); - expect(useConcordanceIndex).toHaveBeenLastCalledWith( + expect(useSourceTextReader).toHaveBeenLastCalledWith( expect.objectContaining({ projectId: testProjectId, writingSystem: 'hbo', liveBook: GEN_1_1_BOOK, enabled: true, - shown: true, }), ); }); - it('keeps the concordance index across a wipe that replaces the draft', async () => { - let indexMounts = 0; - const index: ConcordanceIndex = { - status: 'ready', - booksRead: 0, - bookCount: 0, - entries: [], - isPartial: false, - textForms: undefined, - refresh: jest.fn(), - request: jest.fn(), - }; - jest.mocked(useConcordanceIndex).mockImplementation(() => { + it('builds the concordance as shown once it is shown', async () => { + mockBookData({ writingSystem: 'hbo' }); + await act(async () => { + renderLoader(); + }); + + await userEvent.click(screen.getByTestId('tab-toolbar-concordance')); + + expect(useConcordanceEntries).toHaveBeenLastCalledWith( + expect.objectContaining({ shown: true, writingSystem: 'hbo' }), + ); + }); + + it('keeps the source text across a wipe that replaces the draft', async () => { + let readerMounts = 0; + const text = makeSourceText({ status: 'ready' }); + jest.mocked(useSourceTextReader).mockImplementation(() => { // Held in state so a remount of whatever calls the hook shows up as a fresh count. useReactState(() => { - indexMounts += 1; - return indexMounts; + readerMounts += 1; + return readerMounts; }); - return index; + return text; }); await act(async () => { renderLoader(); @@ -4185,10 +4189,10 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('wipe-confirm-all')); expect(interlinearizerMountCount).toBeGreaterThan(viewMounts); - expect(indexMounts).toBe(1); + expect(readerMounts).toBe(1); }); - it('keeps the concordance index once the side panel switches away from it', async () => { + it('keeps reading the text once the side panel switches away from the concordance', async () => { await act(async () => { renderLoader(); }); @@ -4197,7 +4201,7 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('side-panel-tab-catalog')); await userEvent.click(screen.getByTestId('analysis-catalog-close')); - expect(useConcordanceIndex).toHaveBeenLastCalledWith( + expect(useSourceTextReader).toHaveBeenLastCalledWith( expect.objectContaining({ enabled: true }), ); }); @@ -4210,7 +4214,7 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('side-panel-tab-catalog')); - expect(useConcordanceIndex).toHaveBeenLastCalledWith( + expect(useConcordanceEntries).toHaveBeenLastCalledWith( expect.objectContaining({ shown: false }), ); }); @@ -4904,7 +4908,7 @@ function prepareStoreProbeTest(): void { mockLostBoundaries([]); mockStaleAnalyses(); mockProjectBookIds(undefined); - mockUnreadConcordanceIndex(); + mockUnreadSourceText(); mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); jest .mocked(useData) @@ -5012,28 +5016,12 @@ describe('whole-text re-anchoring', () => { mountStoreProbe = false; }); - /** The arguments the loader last handed the index of the source text. */ - function indexArgs(): UseConcordanceIndexArgs { - const { calls } = jest.mocked(useConcordanceIndex).mock; + /** The arguments the loader last handed the reader of the source text. */ + function readerArgs(): UseSourceTextReaderArgs { + const { calls } = jest.mocked(useSourceTextReader).mock; return calls[calls.length - 1][0]; } - /** A one-verse book reading "Alpha beta.", where "beta" begins at offset 6. */ - function alphaBetaBook(bookCode: string): Book { - return { - id: bookCode, - bookRef: bookCode, - textVersion: 'v1', - duplicateVerseIds: [], - segments: [ - makeSegment(`${bookCode} 1:1`, 'Alpha beta.', [ - makeWordToken(`${bookCode} 1:1:0`, 'Alpha'), - makeWordToken(`${bookCode} 1:1:6`, 'beta', 6), - ]), - ], - }; - } - /** Loads a draft approving "beta" at `tokenRef`, as written against "Al beta.". */ async function renderWithApprovalAt(tokenRef: string): Promise { const analysis = analysisApprovingAt(tokenRef, 'beta'); @@ -5043,46 +5031,32 @@ describe('whole-text re-anchoring', () => { it('re-anchors the draft to a book the text read hands on', async () => { await renderWithApprovalAt('EXO 1:1:3'); - - act(() => indexArgs().onBookRead?.(alphaBetaBook('EXO'))); - - expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('EXO 1:1:6'); - }); - - it('leaves the loaded book to its live text rather than a reading of it', async () => { - mockBookData({ book: alphaBetaBook('GEN') }); - await renderWithApprovalAt('GEN 1:1:3'); - const olderReading: Book = { - ...alphaBetaBook('GEN'), + const exodus: Book = { + id: 'EXO', + bookRef: 'EXO', + textVersion: 'v1', + duplicateVerseIds: [], segments: [ - makeSegment('GEN 1:1', 'Al beta.', [ - makeWordToken('GEN 1:1:0', 'Al'), - makeWordToken('GEN 1:1:3', 'beta', 3), + makeSegment('EXO 1:1', 'Alpha beta.', [ + makeWordToken('EXO 1:1:0', 'Alpha'), + makeWordToken('EXO 1:1:6', 'beta', 6), ]), ], }; - act(() => indexArgs().onBookRead?.(olderReading)); + act(() => readerArgs().onBookRead?.(exodus)); - expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('GEN 1:1:6'); + expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('EXO 1:1:6'); }); it('stales the approvals in a book the read text lacks', async () => { await renderWithApprovalAt('EXO 1:1:3'); - act(() => indexArgs().onTextRead?.(['GEN'])); + act(() => readerArgs().onTextRead?.(['GEN'])); expect(probeAnalysis?.tokenAnalysisLinks[0].status).toBe('stale'); }); - it('leaves the approvals in a book the read text holds', async () => { - await renderWithApprovalAt('EXO 1:1:3'); - - act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); - - expect(probeAnalysis?.tokenAnalysisLinks[0].status).toBe('approved'); - }); - /** Opens the catalog and its filter controls. */ async function openCatalogFilters(): Promise { await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); @@ -5101,7 +5075,7 @@ describe('whole-text re-anchoring', () => { it('says stale places are found in every book once the text is read', async () => { await renderWithApprovalAt('EXO 1:1:3'); - act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); + act(() => readerArgs().onTextRead?.(['GEN', 'EXO'])); await openCatalogFilters(); @@ -5112,73 +5086,14 @@ describe('whole-text re-anchoring', () => { ).toBeInTheDocument(); }); - it('says stale places are found only in opened books once the draft is replaced', async () => { - await renderWithApprovalAt('EXO 1:1:3'); - act(() => indexArgs().onTextRead?.(['GEN', 'EXO'])); - await userEvent.click(screen.getByTestId('tab-toolbar-wipe')); - await userEvent.click(screen.getByTestId('wipe-confirm-book')); - - await openCatalogFilters(); - - expect( - screen.getByRole('checkbox', { name: '%interlinearizer_analysisCatalog_filter_stale%' }), - ).toBeInTheDocument(); - }); - it('reads the text again once the draft is replaced', async () => { await renderWithApprovalAt('EXO 1:1:3'); - const { readKey } = indexArgs(); + const { readKey } = readerArgs(); await userEvent.click(screen.getByTestId('tab-toolbar-wipe')); await userEvent.click(screen.getByTestId('wipe-confirm-book')); - expect(indexArgs().readKey).not.toBe(readKey); - }); - - describe('while an import is shown', () => { - /** Shows a stored import in place of a draft approving "beta" at EXO 1:1:3. */ - async function renderImportBesideDraft(): Promise { - const analysis = analysisApprovingAt('EXO 1:1:3', 'beta'); - mockSendCommand.mockImplementation(async (...args) => - JSON.stringify( - args[0] === 'interlinearizer.getProject' - ? { ...STUB_IMPORT_PROJECT, analysis: emptyAnalysis() } - : { ...emptyDraft(testProjectId), analysis }, - ), - ); - await act(async () => - renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }), - ); - await screen.findByTestId('pt9-import-banner'); - } - - /** Lets any write of the draft past its debounce, then whether one was made. */ - function draftWasSaved(): boolean { - act(() => jest.advanceTimersByTime(1000)); - return mockSendCommand.mock.calls.some( - ([command]) => command === 'interlinearizer.saveDraft', - ); - } - - afterEach(() => jest.useRealTimers()); - - it('re-anchors the draft to no book the text read hands on', async () => { - await renderImportBesideDraft(); - jest.useFakeTimers(); - - act(() => indexArgs().onBookRead?.(alphaBetaBook('EXO'))); - - expect(draftWasSaved()).toBe(false); - }); - - it('stales nothing in the draft for a book the read text lacks', async () => { - await renderImportBesideDraft(); - jest.useFakeTimers(); - - act(() => indexArgs().onTextRead?.(['GEN'])); - - expect(draftWasSaved()).toBe(false); - }); + expect(readerArgs().readKey).not.toBe(readKey); }); }); diff --git a/src/__tests__/components/SourceTextContext.test.tsx b/src/__tests__/components/SourceTextContext.test.tsx new file mode 100644 index 000000000..9df070b6c --- /dev/null +++ b/src/__tests__/components/SourceTextContext.test.tsx @@ -0,0 +1,92 @@ +/// + +import { renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { + SourceTextProvider, + useConcordanceEntriesContext, + useSourceTextContext, +} from '../../components/SourceTextContext'; +import useConcordanceEntries from '../../hooks/useConcordanceEntries'; +import useSourceTextReader, { type SourceText } from '../../hooks/useSourceTextReader'; +import type { ConcordanceEntry } from '../../utils/concordance'; +import { GEN_1_1_BOOK } from '../test-helpers'; + +// Reading books and building entries are the hooks' own concerns; the provider only hands on what +// they build. +jest.mock('../../hooks/useConcordanceEntries'); +jest.mock('../../hooks/useSourceTextReader'); + +const TEXT: SourceText = { + status: 'ready', + booksRead: 1, + bookCount: 1, + readings: new Map(), + liveVersions: new Map(), + isPartial: false, + textForms: undefined, + refresh: () => {}, + request: () => {}, +}; + +const ENTRIES: readonly ConcordanceEntry[] = []; + +/** Wraps a hook in a provider with GEN 1:1 live and the concordance shown. */ +function wrapper({ children }: Readonly<{ children: ReactNode }>) { + return ( + + {children} + + ); +} + +beforeEach(() => { + jest.mocked(useSourceTextReader).mockReturnValue(TEXT); + jest.mocked(useConcordanceEntries).mockReturnValue(ENTRIES); +}); + +describe('SourceTextProvider', () => { + it('reads the text from its props and hands it to what it wraps', () => { + const { result } = renderHook(() => useSourceTextContext(), { wrapper }); + + expect(result.current).toBe(TEXT); + expect(useSourceTextReader).toHaveBeenCalledWith({ + enabled: true, + liveBook: GEN_1_1_BOOK, + projectId: 'src', + writingSystem: 'en', + }); + }); + + it('builds the entries from the text it read and hands them to what it wraps', () => { + const { result } = renderHook(() => useConcordanceEntriesContext(), { wrapper }); + + expect(result.current).toBe(ENTRIES); + expect(useConcordanceEntries).toHaveBeenCalledWith({ + text: TEXT, + shown: true, + writingSystem: 'en', + }); + }); +}); + +describe('useSourceTextContext', () => { + it('throws outside a provider', () => { + // React logs the render error before rethrowing it; silenced so the expected throw is quiet. + jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => renderHook(() => useSourceTextContext())).toThrow( + 'useSourceTextContext must be used within its provider', + ); + }); +}); + +describe('useConcordanceEntriesContext', () => { + it('throws outside a provider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => renderHook(() => useConcordanceEntriesContext())).toThrow( + 'useConcordanceEntriesContext must be used within its provider', + ); + }); +}); diff --git a/src/__tests__/components/TextReadingStatus.test.tsx b/src/__tests__/components/TextReadingStatus.test.tsx index 21b4d2db5..87e99df3a 100644 --- a/src/__tests__/components/TextReadingStatus.test.tsx +++ b/src/__tests__/components/TextReadingStatus.test.tsx @@ -5,8 +5,8 @@ import { render, screen } from '@testing-library/react'; import TextReadingStatus from '../../components/TextReadingStatus'; const localizedStrings = { - '%interlinearizer_concordance_loading%': 'Reading {read} of {total}', - '%interlinearizer_concordance_error%': 'The text could not be read.', + '%interlinearizer_textReading_loading%': 'Reading {read} of {total}', + '%interlinearizer_textReading_error%': 'The text could not be read.', }; describe('TextReadingStatus', () => { @@ -14,7 +14,7 @@ describe('TextReadingStatus', () => { render( , ); @@ -27,7 +27,7 @@ describe('TextReadingStatus', () => { render( , ); @@ -40,7 +40,7 @@ describe('TextReadingStatus', () => { render( , ); diff --git a/src/__tests__/hooks/useConcordanceEntries.test.ts b/src/__tests__/hooks/useConcordanceEntries.test.ts new file mode 100644 index 000000000..33da78a50 --- /dev/null +++ b/src/__tests__/hooks/useConcordanceEntries.test.ts @@ -0,0 +1,105 @@ +/// + +import { renderHook } from '@testing-library/react'; +import useConcordanceEntries, { + type UseConcordanceEntriesArgs, +} from '../../hooks/useConcordanceEntries'; +import { buildConcordanceEntries, type BookConcordance } from '../../utils/concordance'; + +// The entries' own contents are the concordance core's concern; this stand-in names each entry by +// the book and text version it was built from, so a test can read off which reading went in. +jest.mock('../../utils/concordance'); + +/** A stand-in index of one book, which the stand-in entries read only the code and version of. */ +function index(book: string, textVersion: string): BookConcordance { + return { book, textVersion, occurrencesByForm: new Map(), spellingCountsByForm: new Map() }; +} + +/** Indexes keyed by book, as the source text holds them. */ +function byBook(...indexes: BookConcordance[]): ReadonlyMap { + return new Map(indexes.map((i) => [i.book, i])); +} + +const READINGS = byBook(index('GEN', 'v1'), index('EXO', 'v1')); + +const baseArgs: UseConcordanceEntriesArgs = { + text: { status: 'ready', readings: READINGS, liveVersions: new Map() }, + shown: true, + writingSystem: 'en', +}; + +beforeEach(() => { + jest.mocked(buildConcordanceEntries).mockImplementation((books) => + [...books].map((b) => ({ + form: `${b.book}@${b.textVersion}`, + displayText: b.book, + occurrences: [], + countByBook: new Map(), + })), + ); +}); + +/** The book and version each entry was built from. */ +function builtFrom(entries: readonly { form: string }[]): string[] { + return entries.map((e) => e.form).sort(); +} + +describe('useConcordanceEntries', () => { + it('builds nothing until every book has been read', () => { + const { result } = renderHook(() => + useConcordanceEntries({ + ...baseArgs, + text: { status: 'loading', readings: undefined, liveVersions: new Map() }, + }), + ); + + expect(result.current).toEqual([]); + }); + + it('builds an entry from every reading', () => { + const { result } = renderHook(() => useConcordanceEntries(baseArgs)); + + expect(builtFrom(result.current)).toEqual(['EXO@v1', 'GEN@v1']); + }); + + it('takes a live version in place of its reading', () => { + const { result } = renderHook(() => + useConcordanceEntries({ + ...baseArgs, + text: { ...baseArgs.text, liveVersions: byBook(index('GEN', 'v2')) }, + }), + ); + + expect(builtFrom(result.current)).toEqual(['EXO@v1', 'GEN@v2']); + }); + + it('holds back live versions while the concordance is hidden', () => { + const { result, rerender } = renderHook( + (args: UseConcordanceEntriesArgs) => useConcordanceEntries(args), + { initialProps: { ...baseArgs, shown: false } }, + ); + const builds = jest.mocked(buildConcordanceEntries).mock.calls.length; + + rerender({ + ...baseArgs, + shown: false, + text: { ...baseArgs.text, liveVersions: byBook(index('GEN', 'v2')) }, + }); + + expect(jest.mocked(buildConcordanceEntries).mock.calls.length).toBe(builds); + expect(builtFrom(result.current)).toEqual(['EXO@v1', 'GEN@v1']); + }); + + it('takes in live versions held back while hidden once the concordance is shown', () => { + const liveVersions = byBook(index('GEN', 'v2')); + const { result, rerender } = renderHook( + (args: UseConcordanceEntriesArgs) => useConcordanceEntries(args), + { initialProps: { ...baseArgs, shown: false } }, + ); + rerender({ ...baseArgs, shown: false, text: { ...baseArgs.text, liveVersions } }); + + rerender({ ...baseArgs, text: { ...baseArgs.text, liveVersions } }); + + expect(builtFrom(result.current)).toEqual(['EXO@v1', 'GEN@v2']); + }); +}); diff --git a/src/__tests__/hooks/useConcordanceIndex.test.ts b/src/__tests__/hooks/useSourceTextReader.test.ts similarity index 68% rename from src/__tests__/hooks/useConcordanceIndex.test.ts rename to src/__tests__/hooks/useSourceTextReader.test.ts index 6c1272f9e..5140d3325 100644 --- a/src/__tests__/hooks/useConcordanceIndex.test.ts +++ b/src/__tests__/hooks/useSourceTextReader.test.ts @@ -5,11 +5,14 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import type { Book } from 'interlinearizer'; import { tokenizeBook } from 'parsers/papi/bookTokenizer'; import { extractBookFromUsj, type UsjDocument } from 'parsers/papi/usjBookExtractor'; -import useConcordanceIndex, { type UseConcordanceIndexArgs } from '../../hooks/useConcordanceIndex'; -import { buildConcordanceEntries, indexBook } from '../../utils/concordance'; +import useSourceTextReader, { + type SourceText, + type UseSourceTextReaderArgs, +} from '../../hooks/useSourceTextReader'; +import { indexBook } from '../../utils/concordance'; import { getMockedPdpGet } from '../test-helpers'; -// The index's own contents are the concordance core's concern; these stand-ins name each entry by +// A book's index is the concordance core's concern; this stand-in names the one form it holds by // the book and text version it was built from, so a test can read off which reading went in. jest.mock('../../utils/concordance'); jest.mock('parsers/papi/bookTokenizer'); @@ -48,12 +51,11 @@ function deferred() { return { promise, resolve: settle }; } -const baseArgs: UseConcordanceIndexArgs = { +const baseArgs: UseSourceTextReaderArgs = { projectId: 'src', writingSystem: 'en', liveBook: undefined, enabled: true, - shown: true, }; beforeEach(() => { @@ -72,26 +74,23 @@ beforeEach(() => { occurrencesByForm: new Map([[`${b.bookRef}@${b.textVersion}`, []]]), spellingCountsByForm: new Map(), })); - jest.mocked(buildConcordanceEntries).mockImplementation((books) => - [...books].map((b) => ({ - form: `${b.book}@${b.textVersion}`, - displayText: b.book, - occurrences: [], - countByBook: new Map(), - })), - ); }); -/** The book and version each entry was built from. */ -function builtFrom(entries: readonly { form: string }[]): string[] { - return entries.map((e) => e.form).sort(); +/** The book and version each of the indexes was built from. */ +function builtFrom(indexes: SourceText['readings']): string[] { + return [...(indexes?.values() ?? [])].map((i) => `${i.book}@${i.textVersion}`).sort(); +} + +/** The forms the text holds, in a stable order. */ +function formsOf(text: SourceText): string[] | undefined { + return text.textForms && [...text.textForms].sort(); } -describe('useConcordanceIndex', () => { - it('reads nothing until the index is wanted', () => { +describe('useSourceTextReader', () => { + it('reads nothing until the text is wanted', () => { serveProject('1', async () => usj('GEN', 'v1')); - const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, enabled: false })); + const { result } = renderHook(() => useSourceTextReader({ ...baseArgs, enabled: false })); expect(result.current.status).toBe('idle'); expect(mockPdpGet).not.toHaveBeenCalled(); @@ -99,41 +98,41 @@ describe('useConcordanceIndex', () => { it('reads the books once they are requested', async () => { serveProject('1', async (id) => usj(id, 'v1')); - const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, enabled: false })); + const { result } = renderHook(() => useSourceTextReader({ ...baseArgs, enabled: false })); act(() => result.current.request()); await waitFor(() => expect(result.current.status).toBe('ready')); - expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v1']); + expect(formsOf(result.current)).toEqual(['GEN@v1']); }); it('reads every book the project marks present', async () => { const getBookUSJ = serveProject('1010', async (id) => usj(id, 'v1')); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); - expect(builtFrom(result.current.entries)).toEqual(['GEN@v1', 'LEV@v1']); + expect(builtFrom(result.current.readings)).toEqual(['GEN@v1', 'LEV@v1']); expect(getBookUSJ.mock.calls.map(([ref]) => ref.book)).toEqual(['GEN', 'LEV']); }); it('ignores present-book flags past the end of the canon', async () => { const getBookUSJ = serveProject(`${'0'.repeat(199)}1`, async (id) => usj(id, 'v1')); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); expect(getBookUSJ).not.toHaveBeenCalled(); }); - it('reports progress and withholds the entries until every book is read', async () => { + it('reports progress and withholds the readings until every book is read', async () => { const exodus = deferred(); serveProject('11', (id) => (id === 'EXO' ? exodus.promise : Promise.resolve(usj(id, 'v1')))); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.booksRead).toBe(1)); - expect(result.current).toMatchObject({ status: 'loading', bookCount: 2, entries: [] }); + expect(result.current).toMatchObject({ status: 'loading', bookCount: 2, readings: undefined }); await act(async () => exodus.resolve(usj('EXO', 'v1'))); @@ -143,10 +142,10 @@ describe('useConcordanceIndex', () => { it('skips a book the project serves no text for', async () => { serveProject('11', async (id) => (id === 'GEN' ? undefined : usj(id, 'v1'))); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v1']); + expect(builtFrom(result.current.readings)).toEqual(['EXO@v1']); expect(logger.warn).toHaveBeenCalled(); }); @@ -156,17 +155,17 @@ describe('useConcordanceIndex', () => { return usj(id, 'v1'); }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v1']); + expect(builtFrom(result.current.readings)).toEqual(['EXO@v1']); }); it('hands on each book it reads', async () => { serveProject('11', async (id) => usj(id, 'v1')); const onBookRead = jest.fn(); - renderHook(() => useConcordanceIndex({ ...baseArgs, onBookRead })); + renderHook(() => useSourceTextReader({ ...baseArgs, onBookRead })); await waitFor(() => expect(onBookRead.mock.calls.map(([read]) => read)).toEqual([ @@ -180,7 +179,7 @@ describe('useConcordanceIndex', () => { serveProject('111', async (id) => (id === 'EXO' ? undefined : usj(id, 'v1'))); const onTextRead = jest.fn(); - renderHook(() => useConcordanceIndex({ ...baseArgs, onTextRead })); + renderHook(() => useSourceTextReader({ ...baseArgs, onTextRead })); await waitFor(() => expect(onTextRead).toHaveBeenCalledWith(['GEN', 'LEV'])); }); @@ -192,7 +191,7 @@ describe('useConcordanceIndex', () => { }); const onTextRead = jest.fn(); - const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, onTextRead })); + const { result } = renderHook(() => useSourceTextReader({ ...baseArgs, onTextRead })); await waitFor(() => expect(result.current.status).toBe('ready')); expect(onTextRead).not.toHaveBeenCalled(); @@ -204,7 +203,7 @@ describe('useConcordanceIndex', () => { return usj(id, 'v1'); }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); expect(result.current.isPartial).toBe(true); @@ -219,7 +218,7 @@ describe('useConcordanceIndex', () => { genesisReads += 1; return genesisReads === 1 ? Promise.reject(new Error('unreadable')) : secondGenesis.promise; }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.isPartial).toBe(true)); act(() => result.current.refresh()); @@ -236,13 +235,13 @@ describe('useConcordanceIndex', () => { if (genesisReads === 1) throw new Error('unreadable'); return usj(id, 'v1'); }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.isPartial).toBe(true)); act(() => result.current.refresh()); await waitFor(() => expect(result.current.textForms).toBeDefined()); - expect([...(result.current.textForms ?? [])].sort()).toEqual(['EXO@v1', 'GEN@v1']); + expect(formsOf(result.current)).toEqual(['EXO@v1', 'GEN@v1']); expect(result.current.isPartial).toBe(false); }); @@ -252,137 +251,134 @@ describe('useConcordanceIndex', () => { return undefined; }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('error')); - expect(result.current.entries).toEqual([]); + expect(result.current.readings).toBeUndefined(); expect(logger.error).toHaveBeenCalled(); }); - it('reports an empty index when no book has text and none failed', async () => { + it('reports an empty text when no book has text and none failed', async () => { serveProject('11', async () => undefined); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); - expect(result.current.entries).toEqual([]); + expect(formsOf(result.current)).toEqual([]); }); it('reports an error when the list of books cannot be read', async () => { mockPdpGet.mockRejectedValue(new Error('no provider')); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('error')); expect(logger.error).toHaveBeenCalled(); }); - it('takes the live book in place of its reading', async () => { + it('keeps a live book newer than its reading as a live version', async () => { serveProject('11', async (id) => usj(id, 'v1')); const { result } = renderHook(() => - useConcordanceIndex({ ...baseArgs, liveBook: book('GEN', 'v2') }), + useSourceTextReader({ ...baseArgs, liveBook: book('GEN', 'v2') }), ); await waitFor(() => expect(result.current.status).toBe('ready')); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v1', 'GEN@v2']); + expect(builtFrom(result.current.liveVersions)).toEqual(['GEN@v2']); }); it('serves the forms of every book, the live one in place of its reading', async () => { serveProject('11', async (id) => usj(id, 'v1')); const { result } = renderHook(() => - useConcordanceIndex({ ...baseArgs, liveBook: book('GEN', 'v2') }), + useSourceTextReader({ ...baseArgs, liveBook: book('GEN', 'v2') }), ); await waitFor(() => expect(result.current.status).toBe('ready')); - expect([...(result.current.textForms ?? [])].sort()).toEqual(['EXO@v1', 'GEN@v2']); + expect(formsOf(result.current)).toEqual(['EXO@v1', 'GEN@v2']); }); it('keeps the last live version of a book the editor moves off', async () => { serveProject('11', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), { initialProps: { ...baseArgs, liveBook: book('GEN', 'v2') } }, ); await waitFor(() => expect(result.current.status).toBe('ready')); rerender({ ...baseArgs, liveBook: book('EXO', 'v2') }); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v2', 'GEN@v2']); + expect(builtFrom(result.current.liveVersions)).toEqual(['EXO@v2', 'GEN@v2']); }); - it('holds back live-book edits while the concordance is hidden', async () => { + it('takes live-book edits into the forms', async () => { serveProject('1', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: { ...baseArgs, shown: false } }, + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), + { initialProps: baseArgs }, ); await waitFor(() => expect(result.current.status).toBe('ready')); - const builds = jest.mocked(buildConcordanceEntries).mock.calls.length; - rerender({ ...baseArgs, shown: false, liveBook: book('GEN', 'v2') }); + rerender({ ...baseArgs, liveBook: book('GEN', 'v2') }); - expect(jest.mocked(buildConcordanceEntries).mock.calls.length).toBe(builds); - expect(builtFrom(result.current.entries)).toEqual(['GEN@v1']); + expect(formsOf(result.current)).toEqual(['GEN@v2']); }); - it('takes live-book edits into the forms while the concordance is hidden', async () => { + it('keeps no live version for a live book whose text matches its reading', async () => { serveProject('1', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: { ...baseArgs, shown: false } }, + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), + { initialProps: baseArgs }, ); await waitFor(() => expect(result.current.status).toBe('ready')); + const { liveVersions } = result.current; - rerender({ ...baseArgs, shown: false, liveBook: book('GEN', 'v2') }); + rerender({ ...baseArgs, liveBook: book('GEN', 'v1') }); - expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v2']); + expect(result.current.liveVersions).toBe(liveVersions); }); - it('takes in edits held back while hidden once the concordance is shown', async () => { - serveProject('1', async (id) => usj(id, 'v1')); - const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: { ...baseArgs, shown: false } }, - ); + it('reads every book again on refresh', async () => { + let version = 'v1'; + serveProject('111', async (id) => usj(id, version)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(result.current.status).toBe('ready')); - rerender({ ...baseArgs, shown: false, liveBook: book('GEN', 'v2') }); - rerender({ ...baseArgs, liveBook: book('GEN', 'v2') }); + version = 'v3'; + act(() => result.current.refresh()); - expect(builtFrom(result.current.entries)).toEqual(['GEN@v2']); + await waitFor(() => + expect(builtFrom(result.current.readings)).toEqual(['EXO@v3', 'GEN@v3', 'LEV@v3']), + ); }); - it('does not rebuild for a live book whose text matches its reading', async () => { - serveProject('1', async (id) => usj(id, 'v1')); + it("keeps only the live book's live version on refresh", async () => { + serveProject('111', async (id) => usj(id, 'v1')); const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: baseArgs }, + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), + { initialProps: { ...baseArgs, liveBook: book('GEN', 'v2') } }, ); await waitFor(() => expect(result.current.status).toBe('ready')); - const builds = jest.mocked(buildConcordanceEntries).mock.calls.length; + rerender({ ...baseArgs, liveBook: book('EXO', 'v2') }); - rerender({ ...baseArgs, liveBook: book('GEN', 'v1') }); + act(() => result.current.refresh()); - expect(jest.mocked(buildConcordanceEntries).mock.calls.length).toBe(builds); + expect(builtFrom(result.current.liveVersions)).toEqual(['EXO@v2']); }); - it('reads every book again on refresh, keeping the live one', async () => { - let version = 'v1'; - serveProject('111', async (id) => usj(id, version)); + it('forgets live versions of other books on refresh when no book is live', async () => { + serveProject('1', async (id) => usj(id, 'v1')); + const genesisLive: UseSourceTextReaderArgs = { ...baseArgs, liveBook: book('GEN', 'v2') }; const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: { ...baseArgs, liveBook: book('GEN', 'v2') } }, + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), + { initialProps: genesisLive }, ); await waitFor(() => expect(result.current.status).toBe('ready')); - rerender({ ...baseArgs, liveBook: book('EXO', 'v2') }); + rerender(baseArgs); - version = 'v3'; act(() => result.current.refresh()); - await waitFor(() => expect(result.current.status).toBe('ready')); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v2', 'GEN@v3', 'LEV@v3']); + expect(builtFrom(result.current.liveVersions)).toEqual([]); }); it('abandons a read in progress when refreshed', async () => { @@ -393,7 +389,7 @@ describe('useConcordanceIndex', () => { genesisReads += 1; return genesisReads === 1 ? firstGenesis.promise : Promise.resolve(usj(id, 'v2')); }); - const { result } = renderHook(() => useConcordanceIndex(baseArgs)); + const { result } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(genesisReads).toBe(1)); act(() => result.current.refresh()); @@ -401,7 +397,7 @@ describe('useConcordanceIndex', () => { await act(async () => firstGenesis.resolve(usj('GEN', 'stale'))); expect(result.current.status).toBe('ready'); - expect(builtFrom(result.current.entries)).toEqual(['EXO@v1', 'GEN@v2']); + expect(builtFrom(result.current.readings)).toEqual(['EXO@v1', 'GEN@v2']); }); it('hands on no book from a read abandoned by a refresh', async () => { @@ -412,7 +408,7 @@ describe('useConcordanceIndex', () => { return genesisReads === 1 ? firstGenesis.promise : Promise.resolve(usj('GEN', 'v2')); }); const onBookRead = jest.fn(); - const { result } = renderHook(() => useConcordanceIndex({ ...baseArgs, onBookRead })); + const { result } = renderHook(() => useSourceTextReader({ ...baseArgs, onBookRead })); await waitFor(() => expect(genesisReads).toBe(1)); act(() => result.current.refresh()); await waitFor(() => expect(result.current.status).toBe('ready')); @@ -426,7 +422,7 @@ describe('useConcordanceIndex', () => { let version = 'v1'; serveProject('1', async (id) => usj(id, version)); const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), + (args: UseSourceTextReaderArgs) => useSourceTextReader(args), { initialProps: { ...baseArgs, readKey: 1 } }, ); await waitFor(() => expect(result.current.status).toBe('ready')); @@ -434,12 +430,12 @@ describe('useConcordanceIndex', () => { rerender({ ...baseArgs, readKey: 2 }); - await waitFor(() => expect([...(result.current.textForms ?? [])]).toEqual(['GEN@v2'])); + await waitFor(() => expect(formsOf(result.current)).toEqual(['GEN@v2'])); }); - it('reads nothing when its read key changes before the index is wanted', () => { + it('reads nothing when its read key changes before the text is wanted', () => { const getBookUSJ = serveProject('1', async (id) => usj(id, 'v1')); - const { rerender } = renderHook((args: UseConcordanceIndexArgs) => useConcordanceIndex(args), { + const { rerender } = renderHook((args: UseSourceTextReaderArgs) => useSourceTextReader(args), { initialProps: { ...baseArgs, enabled: false, readKey: 1 }, }); @@ -448,28 +444,11 @@ describe('useConcordanceIndex', () => { expect(getBookUSJ).not.toHaveBeenCalled(); }); - it('forgets live versions of other books on refresh when no book is live', async () => { - let version = 'v1'; - serveProject('1', async (id) => usj(id, version)); - const genesisLive: UseConcordanceIndexArgs = { ...baseArgs, liveBook: book('GEN', 'v2') }; - const { result, rerender } = renderHook( - (args: UseConcordanceIndexArgs) => useConcordanceIndex(args), - { initialProps: genesisLive }, - ); - await waitFor(() => expect(result.current.status).toBe('ready')); - rerender(baseArgs); - - version = 'v3'; - act(() => result.current.refresh()); - - await waitFor(() => expect(builtFrom(result.current.entries)).toEqual(['GEN@v3'])); - }); - it('abandons listing the books when unmounted', async () => { const booksPresent = deferred(); const getBookUSJ = jest.fn(); mockPdpGet.mockResolvedValue({ getSetting: () => booksPresent.promise, getBookUSJ }); - const { unmount } = renderHook(() => useConcordanceIndex(baseArgs)); + const { unmount } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(mockPdpGet).toHaveBeenCalled()); unmount(); @@ -486,7 +465,7 @@ describe('useConcordanceIndex', () => { fail = reject; }), }); - const { unmount } = renderHook(() => useConcordanceIndex(baseArgs)); + const { unmount } = renderHook(() => useSourceTextReader(baseArgs)); await waitFor(() => expect(mockPdpGet).toHaveBeenCalled()); unmount(); diff --git a/src/__tests__/hooks/useWholeTextReanchor.test.ts b/src/__tests__/hooks/useWholeTextReanchor.test.ts new file mode 100644 index 000000000..046f940f6 --- /dev/null +++ b/src/__tests__/hooks/useWholeTextReanchor.test.ts @@ -0,0 +1,183 @@ +/// + +import { act, renderHook } from '@testing-library/react'; +import type { Book, DraftProject } from 'interlinearizer'; +import useWholeTextReanchor, { + type UseWholeTextReanchorArgs, +} from '../../hooks/useWholeTextReanchor'; +import { emptyAnalysis, emptyDraft } from '../../types/empty-factories'; +import { reanchorDraftToBook, reanchorDraftToMissingBook } from '../../utils/reanchor-draft'; +import { FIXTURE_STAMPS } from '../test-helpers'; + +// What a pass does to the draft is the re-anchoring core's concern; these tests check only which +// pass each book is handed. +jest.mock('../../utils/reanchor-draft'); + +const toBookPass = jest.fn(); +const toMissingBookPass = jest.fn(); + +/** A draft approving a word at each of `tokenRefs`. */ +function draftLinkingInto(...tokenRefs: string[]): DraftProject { + return { + ...emptyDraft('src'), + analysis: { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'beta' }], + tokenAnalysisLinks: tokenRefs.map((tokenRef) => ({ + ...FIXTURE_STAMPS, + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef, surfaceText: 'beta' }, + })), + }, + }; +} + +/** A stand-in reading of a book, which the hook reads only the code of. */ +function book(bookRef: string): Book { + return { id: bookRef, bookRef, textVersion: 'v1', segments: [], duplicateVerseIds: [] }; +} + +let reanchorBook: jest.Mock; + +/** Arguments for a loaded draft approving a word in Exodus, with Genesis loaded. */ +function makeArgs(overrides: Partial = {}): UseWholeTextReanchorArgs { + return { + loadedBookCode: 'GEN', + isImportView: false, + isDraftLoading: false, + draftVersion: 1, + getDraftSnapshot: () => draftLinkingInto('EXO 1:1:3'), + reanchorBook, + ...overrides, + }; +} + +beforeEach(() => { + reanchorBook = jest.fn(); + jest.mocked(reanchorDraftToBook).mockReturnValue(toBookPass); + jest.mocked(reanchorDraftToMissingBook).mockReturnValue(toMissingBookPass); +}); + +describe('useWholeTextReanchor', () => { + describe('readKey', () => { + it('is the draft version once the draft has loaded', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ draftVersion: 3 }))); + + expect(result.current.readKey).toBe(3); + }); + + it('is undefined while the draft loads', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ isDraftLoading: true }))); + + expect(result.current.readKey).toBeUndefined(); + }); + + it('is undefined while an import is shown', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ isImportView: true }))); + + expect(result.current.readKey).toBeUndefined(); + }); + }); + + describe('onBookRead', () => { + it('re-anchors the draft to a book it links into', () => { + const exodus = book('EXO'); + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + act(() => result.current.onBookRead(exodus)); + + expect(reanchorDraftToBook).toHaveBeenCalledWith(exodus); + expect(reanchorBook).toHaveBeenCalledWith('EXO', toBookPass); + }); + + it('leaves the loaded book to its live text', () => { + const { result } = renderHook(() => + useWholeTextReanchor(makeArgs({ loadedBookCode: 'EXO' })), + ); + + act(() => result.current.onBookRead(book('EXO'))); + + expect(reanchorBook).not.toHaveBeenCalled(); + }); + + it('leaves a book the draft has no links in', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + act(() => result.current.onBookRead(book('LEV'))); + + expect(reanchorBook).not.toHaveBeenCalled(); + }); + + it('leaves the draft alone while an import is shown', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ isImportView: true }))); + + act(() => result.current.onBookRead(book('EXO'))); + + expect(reanchorBook).not.toHaveBeenCalled(); + }); + }); + + describe('onTextRead', () => { + it('stales the approvals in a book the text lacks', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + act(() => result.current.onTextRead(['GEN'])); + + expect(reanchorDraftToMissingBook).toHaveBeenCalledWith('EXO'); + expect(reanchorBook).toHaveBeenCalledWith('EXO', toMissingBookPass); + }); + + it('leaves a book the text holds', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + act(() => result.current.onTextRead(['GEN', 'EXO'])); + + expect(reanchorBook).not.toHaveBeenCalled(); + }); + + it('leaves the draft alone while an import is shown', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ isImportView: true }))); + + act(() => result.current.onTextRead(['GEN'])); + + expect(reanchorBook).not.toHaveBeenCalled(); + }); + }); + + describe('staleCoversDraft', () => { + it('is false until the text is read', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + expect(result.current.staleCoversDraft).toBe(false); + }); + + it('is true once the text is read', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs())); + + act(() => result.current.onTextRead(['GEN', 'EXO'])); + + expect(result.current.staleCoversDraft).toBe(true); + }); + + it('is false again once the draft is replaced', () => { + const { result, rerender } = renderHook( + (args: UseWholeTextReanchorArgs) => useWholeTextReanchor(args), + { initialProps: makeArgs() }, + ); + act(() => result.current.onTextRead(['GEN', 'EXO'])); + + rerender(makeArgs({ draftVersion: 2 })); + + expect(result.current.staleCoversDraft).toBe(false); + }); + + it('stays false for a text read while an import is shown', () => { + const { result } = renderHook(() => useWholeTextReanchor(makeArgs({ isImportView: true }))); + + act(() => result.current.onTextRead(['GEN', 'EXO'])); + + expect(result.current.staleCoversDraft).toBe(false); + }); + }); +}); diff --git a/src/__tests__/utils/verse-ref.test.ts b/src/__tests__/utils/verse-ref.test.ts index dd62fca3c..2fff1bac6 100644 --- a/src/__tests__/utils/verse-ref.test.ts +++ b/src/__tests__/utils/verse-ref.test.ts @@ -2,7 +2,7 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Segment } from 'interlinearizer'; -import { editVerse, firstVerseNumber, segmentContainsVerse } from '../../utils/verse-ref'; +import { firstVerseNumber, segmentContainsVerse, verseOfId } from '../../utils/verse-ref'; import { makeVerseBook } from '../test-helpers'; /** @@ -207,20 +207,20 @@ describe('firstVerseNumber', () => { }); }); -describe('editVerse', () => { +describe('verseOfId', () => { it("names a token's verse", () => { - expect(editVerse('LUK 2:4:7')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); + expect(verseOfId('LUK 2:4:7')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); }); it("names a verse segment's verse", () => { - expect(editVerse('LUK 2:4')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); + expect(verseOfId('LUK 2:4')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); }); it('names the verse a heading is filed under', () => { - expect(editVerse('LUK 2:4/s1')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); + expect(verseOfId('LUK 2:4/s1')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); }); it('names the verse a heading is filed under for a token in it', () => { - expect(editVerse('LUK 2:4/s1:0')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); + expect(verseOfId('LUK 2:4/s1:0')).toEqual({ book: 'LUK', chapterNum: 2, verseNum: 4 }); }); }); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 0c3f54e64..dbc48fafb 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -32,7 +32,7 @@ import CatalogList from './CatalogList'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import CatalogSelectionBar, { SELECTION_BAR_STRING_KEYS } from './CatalogSelectionBar'; import SidePanelTabs, { SIDE_PANEL_TAB_STRING_KEYS } from './SidePanelTabs'; -import { useConcordanceIndexContext } from './ConcordanceIndexContext'; +import { useSourceTextContext } from './SourceTextContext'; import TextReadingStatus, { TEXT_READING_STRING_KEYS } from './TextReadingStatus'; import { useInterlinearNav } from './InterlinearNavContext'; import useDraftDiscardGate from '../hooks/useDraftDiscardGate'; @@ -222,13 +222,13 @@ export default function AnalysisCatalogPanel({ */ const [chosenFilters, setFilters] = useState({}); - const textIndex = useConcordanceIndexContext(); - const { request: requestText, textForms } = textIndex; + const sourceText = useSourceTextContext(); + const { request: requestText, textForms } = sourceText; /** Records the reader's filters, asking for the text to be read once one needs it. */ const handleFiltersChange = useCallback( (next: CatalogFilters) => { - if (next.notInText || next.stale) requestText(); + if (next.stale) requestText(); setFilters(next); }, [requestText], @@ -267,9 +267,14 @@ export default function AnalysisCatalogPanel({ const rows = useMemo(() => applyCatalogQuery(catalogRows, query), [catalogRows, query]); - /** Whether the listing waits on the text, a filter needing it before it is read. */ + /** Whether the not-in-text filter is on before the whole text has been read. */ const isAwaitingText = filters.notInText && !textForms; + // Covers every way filters get set, not just the controls. + useEffect(() => { + if (filters.notInText) requestText(); + }, [filters.notInText, requestText]); + /** * The current book's name key, asked for separately from {@link STRING_KEYS} so that changing book * re-resolves this alone rather than every string the panel shows. @@ -1074,8 +1079,8 @@ export default function AnalysisCatalogPanel({ {isAwaitingText && ( )} diff --git a/src/components/CatalogFilterPopover.tsx b/src/components/CatalogFilterPopover.tsx index 95b7c067d..3b4d78c2a 100644 --- a/src/components/CatalogFilterPopover.tsx +++ b/src/components/CatalogFilterPopover.tsx @@ -16,7 +16,11 @@ import { import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; import { useId, useState } from 'react'; import type { Confidence } from 'interlinearizer'; -import type { CatalogFacets, CatalogFilters } from '../utils/analysis-query'; +import { + TOGGLE_FILTER_KEYS, + type CatalogFacets, + type CatalogFilters, +} from '../utils/analysis-query'; /** * The name each confidence level is offered under. Confidence is a closed vocabulary, unlike a part @@ -293,13 +297,7 @@ export default function CatalogFilterPopover({ filters.confidence, ...Object.values(filters.features ?? {}), ].filter((selected) => selected?.length).length + - [ - filters.missingGloss, - filters.morphemes, - filters.zeroUsages, - filters.stale, - filters.notInText, - ].filter(Boolean).length; + TOGGLE_FILTER_KEYS.filter((key) => filters[key]).length; return ( diff --git a/src/components/ConcordanceIndexContext.tsx b/src/components/ConcordanceIndexContext.tsx deleted file mode 100644 index dc9f79d4b..000000000 --- a/src/components/ConcordanceIndexContext.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import { createContext, useContext, type ReactNode } from 'react'; -import useConcordanceIndex, { - type ConcordanceIndex, - type UseConcordanceIndexArgs, -} from '../hooks/useConcordanceIndex'; - -/** The concordance's index of the source text. */ -export const ConcordanceIndexContext = createContext(undefined); - -/** Builds the concordance's index, re-rendering only the components below that read it. */ -export function ConcordanceIndexProvider({ - children, - ...args -}: Readonly) { - const index = useConcordanceIndex(args); - return ( - {children} - ); -} - -/** - * Reads the concordance's index. - * - * @throws When called outside a {@link ConcordanceIndexProvider}. - */ -export function useConcordanceIndexContext(): ConcordanceIndex { - const index = useContext(ConcordanceIndexContext); - if (!index) throw new Error('useConcordanceIndexContext must be used within its provider'); - return index; -} diff --git a/src/components/ConcordancePanel.tsx b/src/components/ConcordancePanel.tsx index d54f30214..6fa012bff 100644 --- a/src/components/ConcordancePanel.tsx +++ b/src/components/ConcordancePanel.tsx @@ -12,7 +12,7 @@ import { import { formatReplacementString } from 'platform-bible-utils'; import { useCallback, useMemo, useState, type ReactNode } from 'react'; import { useAnalysis, useAnalysisLanguage } from './AnalysisStore'; -import { useConcordanceIndexContext } from './ConcordanceIndexContext'; +import { useConcordanceEntriesContext, useSourceTextContext } from './SourceTextContext'; import ConcordanceRowView, { CONCORDANCE_ROW_STRING_KEYS } from './ConcordanceRowView'; import { useInterlinearNav } from './InterlinearNavContext'; import SidePanelTabs, { SIDE_PANEL_TAB_STRING_KEYS } from './SidePanelTabs'; @@ -69,7 +69,8 @@ export default function ConcordancePanel({ const refreshTooltip = tooltipContentOrUndefined( resolvedOrEmpty(localizedStrings['%interlinearizer_concordance_refresh%']), ); - const index = useConcordanceIndexContext(); + const text = useSourceTextContext(); + const entries = useConcordanceEntriesContext(); const analysis = useAnalysis(); const analysisLanguage = useAnalysisLanguage(); @@ -83,11 +84,7 @@ export default function ConcordancePanel({ ); // Keyed on the status so a refresh, which passes through loading, starts the window over. - const { - windowRows: windowEntries, - scrollRef, - sentinelRef, - } = useRowWindow(index.entries, index.status); + const { windowRows: windowEntries, scrollRef, sentinelRef } = useRowWindow(entries, text.status); // Joined only for the mounted rows: a join over every form walks every occurrence in the text, // too much to repeat on each gloss written beside the panel. @@ -147,11 +144,11 @@ export default function ConcordancePanel({ ); let body: ReactNode; - if (index.status !== 'ready') { + if (text.status !== 'ready') { body = ( - + ); - } else if (index.entries.length === 0) { + } else if (entries.length === 0) { body = ( @@ -223,7 +220,7 @@ export default function ConcordancePanel({ - {index.isPartial && ( + {text.isPartial && (

    (); - const staleCoversDraft = readingTarget !== undefined && textReanchoredFor === readingTarget; - - /** Re-anchors the draft to a book the whole-text read hands on, where it has anything there. */ - const handleBookRead = useCallback( - (read: Book) => { - if (read.bookRef === loadedBookCodeRef.current) return; - const analysis = - readingTargetRef.current === undefined ? undefined : getDraftSnapshot()?.analysis; - if (!analysis || !booksLinkedIn(analysis).has(read.bookRef)) return; - reanchorBook(read.bookRef, reanchorDraftToBook(read)); - }, - [getDraftSnapshot, reanchorBook], - ); - - /** - * Re-anchors the draft to the absence of every book it has anything in that the text lacks, - * completing its re-anchoring to the whole text. - */ - const handleTextRead = useCallback( - (bookIds: readonly string[]) => { - const target = readingTargetRef.current; - const analysis = target === undefined ? undefined : getDraftSnapshot()?.analysis; - if (!analysis) return; - const present = new Set(bookIds); - booksLinkedIn(analysis).forEach((bookCode) => { - if (!present.has(bookCode)) reanchorBook(bookCode, reanchorDraftToMissingBook(bookCode)); - }); - setTextReanchoredFor(target); - }, - [getDraftSnapshot, reanchorBook], - ); + const wholeTextReanchor = useWholeTextReanchor({ + loadedBookCode: verseBook?.bookRef, + isImportView, + isDraftLoading, + draftVersion, + getDraftSnapshot, + reanchorBook, + }); /** * The book the views render: the verse-tokenized book re-grouped into the user's custom segments. @@ -927,10 +891,7 @@ function InterlinearizerLoaderInner({ DEFAULT_SIDE_PANEL_LAYOUT, ); - /** - * Whether the concordance has been shown in this tab, one of the things that starts the text - * being read. - */ + /** Whether the concordance has been shown in this tab. */ const [concordanceWanted, setConcordanceWanted] = useState(sidePanel === 'concordance'); if (sidePanel === 'concordance' && !concordanceWanted) setConcordanceWanted(true); @@ -941,8 +902,7 @@ function InterlinearizerLoaderInner({ * 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; + const modalRef = useLatestRef(modal); /** Whether the destructive wipe dialog (book / whole-draft scope picker) is open. */ const [wipeModalOpen, setWipeModalOpen] = useState(false); @@ -1004,7 +964,7 @@ function InterlinearizerLoaderInner({ ); if (step?.location) { requestFocusToken(step.location); - navigate(editVerse(step.location)); + navigate(verseOfId(step.location)); } else if (step?.summary) { const { kind, ...replacers } = step.summary; if ('analysisId' in replacers) { @@ -1271,7 +1231,7 @@ function InterlinearizerLoaderInner({ .send({ message: '%interlinearizer_error_load_projects_failed%', severity: 'error' }) .catch(() => {}); } - }, [pt9ImportedId, fetchSummary, setActiveProject]); + }, [pt9ImportedId, fetchSummary, setActiveProject, modalRef]); /** * Dismisses the import modal: a first import returns to the select modal it came from; every @@ -1542,12 +1502,11 @@ function InterlinearizerLoaderInner({ * runs. Restoring as the flag turns therefore reaches the group while it still knows only of the * view, taking the WebView down. */ - const sidePanelLayoutRef = useRef(sidePanelLayout); - sidePanelLayoutRef.current = sidePanelLayout; + const sidePanelLayoutRef = useLatestRef(sidePanelLayout); useEffect(() => { if (sidePanel !== 'closed' && sidePanelRegistered) sidePanelGroupRef.current?.setLayout(sidePanelLayoutRef.current); - }, [sidePanel, sidePanelRegistered]); + }, [sidePanel, sidePanelRegistered, sidePanelLayoutRef]); const sidePanelResizeRef = usePanelResizeKeys( /* v8 ignore next -- every stored layout names the side panel, the default included */ @@ -1778,7 +1737,7 @@ function InterlinearizerLoaderInner({ onShowConcordance={handleShowConcordance} showMorphology={showMorphology} sourceLanguageTag={writingSystem} - staleCoversDraft={staleCoversDraft} + staleCoversDraft={wholeTextReanchor.staleCoversDraft} /> ) : ( {/* Above the keyed stores in the view area, so the source text is read once per tab. */} - {viewArea} - + (undefined); + +/** The concordance's entries across the source text. */ +export const ConcordanceEntriesContext = createContext( + undefined, +); + +/** + * Reads the source text and builds the concordance's entries from it, re-rendering only the + * components below that read either. + */ +export function SourceTextProvider({ + children, + shown, + ...args +}: Readonly< + UseSourceTextReaderArgs & { + /** Whether the concordance is on screen; its entries take in live-book edits only while it is. */ + shown: boolean; + children: ReactNode; + } +>) { + const text = useSourceTextReader(args); + const entries = useConcordanceEntries({ text, shown, writingSystem: args.writingSystem }); + return ( + + + {children} + + + ); +} + +/** + * Reads the source text. + * + * @throws When called outside a {@link SourceTextProvider}. + */ +export function useSourceTextContext(): SourceText { + const text = useContext(SourceTextContext); + if (!text) throw new Error('useSourceTextContext must be used within its provider'); + return text; +} + +/** + * Reads the concordance's entries. + * + * @throws When called outside a {@link SourceTextProvider}. + */ +export function useConcordanceEntriesContext(): readonly ConcordanceEntry[] { + const entries = useContext(ConcordanceEntriesContext); + if (!entries) throw new Error('useConcordanceEntriesContext must be used within its provider'); + return entries; +} diff --git a/src/components/TextReadingStatus.tsx b/src/components/TextReadingStatus.tsx index 4c74ee18c..83ea2a5e2 100644 --- a/src/components/TextReadingStatus.tsx +++ b/src/components/TextReadingStatus.tsx @@ -1,16 +1,16 @@ import { EmptyState, Spinner } from 'platform-bible-react'; import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; -import type { ConcordanceIndex } from '../hooks/useConcordanceIndex'; +import type { SourceText } from '../hooks/useSourceTextReader'; /** Localized string keys {@link TextReadingStatus} renders. */ export const TEXT_READING_STRING_KEYS = [ - '%interlinearizer_concordance_loading%', - '%interlinearizer_concordance_error%', + '%interlinearizer_textReading_loading%', + '%interlinearizer_textReading_error%', ] as const satisfies `%${string}%`[]; /** Props for {@link TextReadingStatus}. */ type TextReadingStatusProps = Readonly<{ - index: Pick; + text: Pick; /** Prefixes the `-loading` and `-error` test ids the status is found by. */ idPrefix: string; /** Resolved localizations covering at least {@link TEXT_READING_STRING_KEYS}. */ @@ -19,16 +19,16 @@ type TextReadingStatusProps = Readonly<{ /** How far reading the source text has got, or that it could not be read. */ export default function TextReadingStatus({ - index, + text, idPrefix, localizedStrings, }: TextReadingStatusProps) { - if (index.status === 'error' || index.isPartial) { + if (text.status === 'error' || text.isPartial) { return ( ); } @@ -39,9 +39,9 @@ export default function TextReadingStatus({ role="status" > - {formatReplacementString(localizedStrings['%interlinearizer_concordance_loading%'], { - read: index.booksRead, - total: index.bookCount, + {formatReplacementString(localizedStrings['%interlinearizer_textReading_loading%'], { + read: text.booksRead, + total: text.bookCount, })}

    ); diff --git a/src/components/__mocks__/TextReadingStatus.tsx b/src/components/__mocks__/TextReadingStatus.tsx index 1e6f81c7b..49f393b93 100644 --- a/src/components/__mocks__/TextReadingStatus.tsx +++ b/src/components/__mocks__/TextReadingStatus.tsx @@ -1,14 +1,14 @@ -import type { ConcordanceIndex } from '../../hooks/useConcordanceIndex'; +import type { SourceText } from '../../hooks/useSourceTextReader'; export const { TEXT_READING_STRING_KEYS } = jest.requireActual('../TextReadingStatus'); -/** Stands in for the reading status, exposing the index state it was handed. */ +/** Stands in for the reading status, exposing the reading state it was handed. */ export default function TextReadingStatus({ - index, -}: Readonly<{ index: Pick }>) { + text, +}: Readonly<{ text: Pick }>) { return ( -

    - {`${index.booksRead} of ${index.bookCount}`} +

    + {`${text.booksRead} of ${text.bookCount}`}

    ); } diff --git a/src/hooks/useConcordanceEntries.ts b/src/hooks/useConcordanceEntries.ts new file mode 100644 index 000000000..81d4e7a64 --- /dev/null +++ b/src/hooks/useConcordanceEntries.ts @@ -0,0 +1,37 @@ +import { useMemo, useState } from 'react'; +import { buildConcordanceEntries, type ConcordanceEntry } from '../utils/concordance'; +import { collatorForTag } from '../utils/language-tags'; +import type { SourceText } from './useSourceTextReader'; + +/** Arguments for {@link useConcordanceEntries}. */ +export interface UseConcordanceEntriesArgs { + /** The source text the entries are built from. */ + text: Pick; + /** Whether the concordance is on screen; the entries take in live-book edits only while it is. */ + shown: boolean; + /** BCP 47 tag of the source text, which the entries are collated by. */ + writingSystem: string; +} + +/** + * Builds the concordance's entries, one per form across every book, from the source text; empty + * until every book has been read. + */ +export default function useConcordanceEntries({ + text: { status, readings, liveVersions }, + shown, + writingSystem, +}: UseConcordanceEntriesArgs): readonly ConcordanceEntry[] { + const collator = useMemo(() => collatorForTag(writingSystem), [writingSystem]); + + // Held back while hidden: merging every book again is too costly to repeat for edits nobody sees. + const [mergedLiveVersions, setMergedLiveVersions] = useState(liveVersions); + if (shown && mergedLiveVersions !== liveVersions) setMergedLiveVersions(liveVersions); + + return useMemo(() => { + if (status !== 'ready' || !readings) return []; + const books = new Map(readings); + mergedLiveVersions.forEach((index, book) => books.set(book, index)); + return buildConcordanceEntries(books.values(), collator); + }, [status, readings, mergedLiveVersions, collator]); +} diff --git a/src/hooks/useConcordanceIndex.ts b/src/hooks/useSourceTextReader.ts similarity index 65% rename from src/hooks/useConcordanceIndex.ts rename to src/hooks/useSourceTextReader.ts index 1efa320ee..9364163b9 100644 --- a/src/hooks/useConcordanceIndex.ts +++ b/src/hooks/useSourceTextReader.ts @@ -2,35 +2,28 @@ import papi, { logger } from '@papi/frontend'; import { Canon } from '@sillsdev/scripture'; import type { Book } from 'interlinearizer'; import type { IUSJBookProjectDataProvider } from 'platform-scripture'; -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { tokenizeBook } from '../parsers/papi/bookTokenizer'; import { extractBookFromUsj } from '../parsers/papi/usjBookExtractor'; -import { - buildConcordanceEntries, - indexBook, - type BookConcordance, - type ConcordanceEntry, -} from '../utils/concordance'; -import { collatorForTag } from '../utils/language-tags'; +import { indexBook, type BookConcordance } from '../utils/concordance'; +import useLatestRef from './useLatestRef'; /** Where reading the project's books has got to. */ -export type ConcordanceIndexStatus = 'idle' | 'loading' | 'ready' | 'error'; +export type SourceTextStatus = 'idle' | 'loading' | 'ready' | 'error'; -/** Arguments for {@link useConcordanceIndex}. */ -export interface UseConcordanceIndexArgs { - /** Source project whose books are indexed. */ +/** Arguments for {@link useSourceTextReader}. */ +export interface UseSourceTextReaderArgs { + /** Source project whose books are read. */ projectId: string; - /** BCP 47 tag of the source text, which the entries are collated by. */ + /** BCP 47 tag of the source text, which its tokens are labeled with. */ writingSystem: string; /** The book the editor has loaded, which stands in for that book's reading while it is current. */ liveBook: Book | undefined; - /** Whether the index is wanted; nothing is read until it first is, or is requested. */ + /** Whether the text is wanted; nothing is read until it first is, or is requested. */ enabled: boolean; - /** Whether the concordance is on screen; the entries take in live-book edits only while it is. */ - shown: boolean; /** - * Reads every book again, as a refresh does, whenever this changes; a reading begun under an - * earlier value hands nothing on. + * Reads every book again, as a refresh does, whenever this changes; results from a read started + * under an earlier key are dropped. */ readKey?: unknown; /** Receives each book as it is read, before only its index is kept. */ @@ -42,16 +35,18 @@ export interface UseConcordanceIndexArgs { onTextRead?: (bookIds: readonly string[]) => void; } -/** What {@link useConcordanceIndex} hands back. */ -export interface ConcordanceIndex { - status: ConcordanceIndexStatus; +/** What {@link useSourceTextReader} hands back. */ +export interface SourceText { + status: SourceTextStatus; /** How many of the project's books have been read so far. */ booksRead: number; /** How many books the project has; `0` until the list of them has been read. */ bookCount: number; - /** One entry per form across every book; empty until every book has been read. */ - entries: readonly ConcordanceEntry[]; - /** Whether a book failed to read, leaving the entries short of the whole text. */ + /** Each book's index as last read, by book code; `undefined` until a reading completes. */ + readings: ReadonlyMap | undefined; + /** Live versions of books the editor has had loaded, each newer than the book's reading. */ + liveVersions: ReadonlyMap; + /** Whether a book failed to read, leaving the readings short of the whole text. */ isPartial: boolean; /** * Every form the text holds, the live book's as it now reads; `undefined` until every book has @@ -60,7 +55,7 @@ export interface ConcordanceIndex { textForms: ReadonlySet | undefined; /** Reads every book again, the live one aside. */ refresh: () => void; - /** Wants the index from now on, whatever `enabled` says. */ + /** Wants the text from now on, whatever `enabled` says. */ request: () => void; } @@ -96,29 +91,27 @@ async function readBook( } /** - * Indexes every book of a source project, reading them the first time the index is wanted or - * requested and keeping them for as long as the caller stays mounted. + * Reads and indexes every book of a source project, the first time the text is wanted or requested, + * and keeps them for as long as the caller stays mounted. * - * Every book but the live one is a reading taken when the index was built. The live book replaces - * its reading whenever its text changes, and a book the editor moves off keeps the last live - * version it had rather than going back to the older reading. + * Every book but the live one is a reading taken when the text was last read. The live book + * replaces its reading whenever its text changes, and a book the editor moves off keeps the last + * live version it had rather than going back to the older reading. */ -export default function useConcordanceIndex({ +export default function useSourceTextReader({ projectId, writingSystem, liveBook, enabled, - shown, onBookRead, onTextRead, readKey, -}: UseConcordanceIndexArgs): ConcordanceIndex { - const [status, setStatus] = useState('idle'); +}: UseSourceTextReaderArgs): SourceText { + const [status, setStatus] = useState('idle'); const [progress, setProgress] = useState({ booksRead: 0, bookCount: 0 }); const [readings, setReadings] = useState>(); const [isPartial, setIsPartial] = useState(false); - /** Live versions of books the editor has had loaded, each newer than the book's reading. */ const [liveVersions, setLiveVersions] = useState>( () => new Map(), ); @@ -131,25 +124,20 @@ export default function useConcordanceIndex({ // Read through a ref so the tag settling after the first render does not restart the reads: the // tag only labels the tokens, which the index never looks at. - const writingSystemRef = useRef(writingSystem); - writingSystemRef.current = writingSystem; + const writingSystemRef = useLatestRef(writingSystem); // Read through refs so a read in flight calls whichever callbacks are current when it lands. - const onBookReadRef = useRef(onBookRead); - onBookReadRef.current = onBookRead; - const onTextReadRef = useRef(onTextRead); - onTextReadRef.current = onTextRead; - const readKeyRef = useRef(readKey); - readKeyRef.current = readKey; + const onBookReadRef = useLatestRef(onBookRead); + const onTextReadRef = useLatestRef(onTextRead); + const readKeyRef = useLatestRef(readKey); - const readingsRef = useRef(readings); - readingsRef.current = readings; + const readingsRef = useLatestRef(readings); useEffect(() => { if (!wanted) return undefined; let isAbandoned = false; const key = readKeyRef.current; - // The key moves on as soon as it renders, ahead of the cleanup that abandons this read. + // `readKeyRef` updates during render, before this cleanup runs, so check the key too. const isCurrent = () => !isAbandoned && readKeyRef.current === key; setStatus('loading'); setIsPartial(false); @@ -173,10 +161,10 @@ export default function useConcordanceIndex({ try { book = await readBook(usjPdp, bookId, writingSystemRef.current); if (book) read.set(bookId, indexBook(book)); - else logger.warn(`Concordance: project ${projectId} has no text for ${bookId}`); + else logger.warn(`Source text: project ${projectId} has no text for ${bookId}`); } catch (e) { booksFailed += 1; - logger.warn(`Concordance: skipping ${bookId} in project ${projectId}`, e); + logger.warn(`Source text: skipping ${bookId} in project ${projectId}`, e); } if (!isCurrent()) return; if (book) onBookReadRef.current?.(book); @@ -186,9 +174,9 @@ export default function useConcordanceIndex({ }; await Promise.all(Array.from({ length: READ_CONCURRENCY }, readNext)); if (!isCurrent()) return; - // With nothing read, a failure could be hiding text, so an empty list would misreport it. + // With nothing read, a failure could be hiding text, so an empty reading would misreport it. if (read.size === 0 && booksFailed > 0) { - logger.error(`Concordance: no book of project ${projectId} could be read`); + logger.error(`Source text: no book of project ${projectId} could be read`); setStatus('error'); return; } @@ -198,16 +186,16 @@ export default function useConcordanceIndex({ if (booksFailed === 0) onTextReadRef.current?.(bookIds.filter((id) => read.has(id))); } catch (e) { if (!isCurrent()) return; - logger.error(`Concordance: could not list the books of project ${projectId}`, e); + logger.error(`Source text: could not list the books of project ${projectId}`, e); setStatus('error'); } })(); return () => { isAbandoned = true; }; - }, [wanted, projectId, generation]); + }, [wanted, projectId, generation, readKeyRef, writingSystemRef, onBookReadRef, onTextReadRef]); - /** The live book, indexed only once the index is wanted. */ + /** The live book, indexed only once the text is wanted. */ const liveIndex = useMemo( () => (wanted && liveBook ? indexBook(liveBook) : undefined), [wanted, liveBook], @@ -217,20 +205,19 @@ export default function useConcordanceIndex({ if (!liveIndex) return; setLiveVersions((prev) => { const known = prev.get(liveIndex.book) ?? readingsRef.current?.get(liveIndex.book); - // Unchanged text would only rebuild every entry to the same result. + // Unchanged text would only rebuild everything built on the readings to the same result. if (known?.textVersion === liveIndex.textVersion) return prev; return new Map(prev).set(liveIndex.book, liveIndex); }); - }, [liveIndex]); + }, [liveIndex, readingsRef]); - const liveIndexRef = useRef(liveIndex); - liveIndexRef.current = liveIndex; + const liveIndexRef = useLatestRef(liveIndex); const refresh = useCallback(() => { const live = liveIndexRef.current; setLiveVersions(live ? new Map([[live.book, live]]) : new Map()); setGeneration((g) => g + 1); - }, []); + }, [liveIndexRef]); const [readKeyRead, setReadKeyRead] = useState(readKey); if (readKey !== readKeyRead) { @@ -240,19 +227,6 @@ export default function useConcordanceIndex({ const request = useCallback(() => setRequested(true), []); - const collator = useMemo(() => collatorForTag(writingSystem), [writingSystem]); - - // Held back while hidden: merging every book again is too costly to repeat for edits nobody sees. - const [mergedLiveVersions, setMergedLiveVersions] = useState(liveVersions); - if (shown && mergedLiveVersions !== liveVersions) setMergedLiveVersions(liveVersions); - - const entries = useMemo(() => { - if (status !== 'ready' || !readings) return []; - const books = new Map(readings); - mergedLiveVersions.forEach((index, book) => books.set(book, index)); - return buildConcordanceEntries(books.values(), collator); - }, [status, readings, mergedLiveVersions, collator]); - const textForms = useMemo(() => { if (status !== 'ready' || !readings || isPartial) return undefined; const books = new Map(readings); @@ -265,7 +239,16 @@ export default function useConcordanceIndex({ }, [status, readings, isPartial, liveVersions]); return useMemo( - () => ({ status, ...progress, entries, isPartial, textForms, refresh, request }), - [status, progress, entries, isPartial, textForms, refresh, request], + () => ({ + status, + ...progress, + readings, + liveVersions, + isPartial, + textForms, + refresh, + request, + }), + [status, progress, readings, liveVersions, isPartial, textForms, refresh, request], ); } diff --git a/src/hooks/useWholeTextReanchor.ts b/src/hooks/useWholeTextReanchor.ts new file mode 100644 index 000000000..44a9c7ee5 --- /dev/null +++ b/src/hooks/useWholeTextReanchor.ts @@ -0,0 +1,86 @@ +import type { Book, DraftProject } from 'interlinearizer'; +import { useCallback, useState } from 'react'; +import { booksLinkedIn } from '../utils/analysis-book'; +import { reanchorDraftToBook, reanchorDraftToMissingBook } from '../utils/reanchor-draft'; +import type { BookPass } from '../utils/undo-history'; +import type { DraftContent } from './useDraftProject'; +import useLatestRef from './useLatestRef'; + +/** Arguments for {@link useWholeTextReanchor}. */ +export interface UseWholeTextReanchorArgs { + /** The book the view has loaded, which re-anchors to its own live text rather than to a reading. */ + loadedBookCode: string | undefined; + /** Whether an import is shown in place of the draft, which is then left as it is. */ + isImportView: boolean; + /** Whether the draft is still loading, before which there is nothing to re-anchor. */ + isDraftLoading: boolean; + /** Bumped whenever the draft is replaced wholesale. */ + draftVersion: number; + getDraftSnapshot: () => DraftProject | undefined; + reanchorBook: (bookCode: string, pass: BookPass) => void; +} + +/** What {@link useWholeTextReanchor} hands back, for a whole-text read to drive. */ +export interface WholeTextReanchor { + /** + * Draft version the whole-text read re-anchors, `undefined` while loading or showing an import; + * each change reads the text again. + */ + readKey: number | undefined; + /** + * As the whole text is read, re-anchors the draft to each book it links into, except the loaded + * book, which re-anchors to its live text. + */ + onBookRead: (book: Book) => void; + /** + * Once the whole text is read, stales the draft's approvals in books the text lacks and records + * the draft as fully re-anchored. + */ + onTextRead: (bookIds: readonly string[]) => void; + /** Whether every book has been re-anchored for the current draft, rather than only those opened. */ + staleCoversDraft: boolean; +} + +/** Re-anchors the draft to every book of the source text each time the whole text is read. */ +export default function useWholeTextReanchor({ + loadedBookCode, + isImportView, + isDraftLoading, + draftVersion, + getDraftSnapshot, + reanchorBook, +}: UseWholeTextReanchorArgs): WholeTextReanchor { + const loadedBookCodeRef = useLatestRef(loadedBookCode); + const readKey = isImportView || isDraftLoading ? undefined : draftVersion; + const readKeyRef = useLatestRef(readKey); + + /** The draft version every book was last re-anchored for. */ + const [textReanchoredFor, setTextReanchoredFor] = useState(); + const staleCoversDraft = readKey !== undefined && textReanchoredFor === readKey; + + const onBookRead = useCallback( + (read: Book) => { + if (read.bookRef === loadedBookCodeRef.current) return; + const analysis = readKeyRef.current === undefined ? undefined : getDraftSnapshot()?.analysis; + if (!analysis || !booksLinkedIn(analysis).has(read.bookRef)) return; + reanchorBook(read.bookRef, reanchorDraftToBook(read)); + }, + [getDraftSnapshot, reanchorBook, loadedBookCodeRef, readKeyRef], + ); + + const onTextRead = useCallback( + (bookIds: readonly string[]) => { + const target = readKeyRef.current; + const analysis = target === undefined ? undefined : getDraftSnapshot()?.analysis; + if (!analysis) return; + const present = new Set(bookIds); + booksLinkedIn(analysis).forEach((bookCode) => { + if (!present.has(bookCode)) reanchorBook(bookCode, reanchorDraftToMissingBook(bookCode)); + }); + setTextReanchoredFor(target); + }, + [getDraftSnapshot, reanchorBook, readKeyRef], + ); + + return { readKey, onBookRead, onTextRead, staleCoversDraft }; +} diff --git a/src/utils/analysis-query.ts b/src/utils/analysis-query.ts index 851e7c218..52501e40d 100644 --- a/src/utils/analysis-query.ts +++ b/src/utils/analysis-query.ts @@ -112,6 +112,15 @@ export interface CatalogFilters { morphemes?: 'has' | 'lacks'; } +/** The filters set by a single toggle rather than by a selection of values. */ +export const TOGGLE_FILTER_KEYS = [ + 'zeroUsages', + 'stale', + 'notInText', + 'missingGloss', + 'morphemes', +] as const satisfies readonly (keyof CatalogFilters)[]; + /** How the caller narrows and orders the rows. */ export interface CatalogQuery { /** @@ -507,6 +516,15 @@ function retainOffered( return kept.length === 0 ? undefined : kept; } +/** Copies one filter across where `from` sets it, leaving it absent rather than `undefined`. */ +function copyDefinedFilter( + from: CatalogFilters, + to: CatalogFilters, + key: K, +): void { + if (from[key] !== undefined) to[key] = from[key]; +} + /** Whether two feature selections name the same values for the same features. */ function sameFeatureSelections( a: CatalogFilters['features'], @@ -547,16 +565,12 @@ export function reconcileFilters(filters: CatalogFilters, facets: CatalogFacets) // Built field by field rather than by overriding a spread of `filters`: a withdrawn selection is // absent rather than `undefined`, and spreading the original first would keep the stale key. const reconciled: CatalogFilters = { - ...(filters.zeroUsages !== undefined && { zeroUsages: filters.zeroUsages }), - ...(filters.stale !== undefined && { stale: filters.stale }), - ...(filters.notInText !== undefined && { notInText: filters.notInText }), - ...(filters.missingGloss !== undefined && { missingGloss: filters.missingGloss }), - ...(filters.morphemes !== undefined && { morphemes: filters.morphemes }), ...(books && { books }), ...(pos && { pos }), ...(confidence && { confidence }), ...(keptFeatures && { features: keptFeatures }), }; + TOGGLE_FILTER_KEYS.forEach((key) => copyDefinedFilter(filters, reconciled, key)); // The value filters are the only ones a facet can withdraw: the rest are offered unconditionally, // so nothing can strand them. diff --git a/src/utils/stale-free-translations.ts b/src/utils/stale-free-translations.ts index 351862f54..17f682bd9 100644 --- a/src/utils/stale-free-translations.ts +++ b/src/utils/stale-free-translations.ts @@ -2,7 +2,7 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Book, Segment } from 'interlinearizer'; import { bookOfRef } from './analysis-book'; import { verseOfTokenRef } from './reanchor-analysis'; -import { firstVerseNumber, segmentContainsVerse } from './verse-ref'; +import { segmentContainsVerse, verseOfId } from './verse-ref'; /** A free translation whose segment's text has changed since it was written. */ export type StaleFreeTranslation = Readonly<{ @@ -86,17 +86,17 @@ function vanishedHeadingPlaces( ); } -/** - * Reads where a verse id, or that of a heading filed under the verse, falls among the book's - * verses: its chapter, then the first verse it names. - */ -function versePosition(verse: string): readonly [number, number] { - const [chapterPart, versePart] = verse - .slice(verse.indexOf(' ') + 1) - .split('/')[0] - .split(':'); - /* v8 ignore next -- a verse id always names a verse number */ - return [Number(chapterPart), firstVerseNumber(versePart) ?? 0]; +/** A place with the chapter and first verse number of the verse it falls in. */ +type VersePlace = Readonly<{ place: Place; chapterNum: number; verseNum: number }>; + +/** Lists every place in the book in document order, each with the verse it falls in. */ +function placesInDocumentOrder(placesByVerse: ReadonlyMap): VersePlace[] { + return [...placesByVerse] + .flatMap(([verse, places]) => { + const { chapterNum, verseNum } = verseOfId(verse); + return places.map((place) => ({ place, chapterNum, verseNum })); + }) + .sort((a, b) => a.place.order - b.place.order); } /** @@ -104,20 +104,11 @@ function versePosition(verse: string): readonly [number, number] { * of the nearest earlier verse, else the book's first place, else `undefined` for a book with * none. */ -function placeBefore( - verse: string, - placesByVerse: ReadonlyMap, -): Place | undefined { - const [chapter, verseNumber] = versePosition(verse); - const inOrder = [...placesByVerse] - .flatMap(([placeVerse, places]) => { - const [placeChapter, placeVerseNumber] = versePosition(placeVerse); - const isEarlier = - placeChapter < chapter || (placeChapter === chapter && placeVerseNumber < verseNumber); - return places.map((place) => ({ place, isEarlier })); - }) - .sort((a, b) => a.place.order - b.place.order); - return (inOrder.findLast((entry) => entry.isEarlier) ?? inOrder[0])?.place; +function placeBefore(verse: string, inOrder: readonly VersePlace[]): Place | undefined { + const { chapterNum, verseNum } = verseOfId(verse); + const isEarlier = (entry: VersePlace) => + entry.chapterNum < chapterNum || (entry.chapterNum === chapterNum && entry.verseNum < verseNum); + return (inOrder.findLast(isEarlier) ?? inOrder[0])?.place; } /** @@ -133,13 +124,14 @@ export function placeStaleFreeTranslations( book: Book, ): ReadonlyMap { const placesByVerse = indexPlacesByVerse(book); + let inOrder: readonly VersePlace[] | undefined; const placed = stale.flatMap((translation) => { if (bookOfRef(translation.segmentId) !== book.bookRef) return []; const { verse, offset } = positionOf(translation.segmentId); const places = placesByVerse.get(verse) ?? vanishedHeadingPlaces(verse, placesByVerse); const covering = places ? (places.findLast((place) => place.offset <= offset) ?? places[0]) - : placeBefore(verse, placesByVerse); + : placeBefore(verse, (inOrder ??= placesInDocumentOrder(placesByVerse))); return covering ? [{ translation, covering }] : []; }); diff --git a/src/utils/verse-ref.ts b/src/utils/verse-ref.ts index 045c56bd3..6266c3f17 100644 --- a/src/utils/verse-ref.ts +++ b/src/utils/verse-ref.ts @@ -77,14 +77,14 @@ export function toSerializedVerseRef(ref: ScriptureRef): SerializedVerseRef { return { book: ref.book, chapterNum: ref.chapter, verseNum: ref.verse }; } -/** The verse that shows the reader an edit made at `location`, a token ref or segment id. */ -export function editVerse(location: string): SerializedVerseRef { - const space = location.indexOf(' '); - const [chapter, verse] = location.slice(space + 1).split(':'); +/** The verse a token ref or segment id names, a heading's being the verse it is filed under. */ +export function verseOfId(id: string): SerializedVerseRef { + const space = id.indexOf(' '); + const [chapter, verse] = id.slice(space + 1).split(':'); return { - book: location.slice(0, space), + book: id.slice(0, space), chapterNum: Number(chapter), - /* v8 ignore next -- an edit is made in a verse or a heading filed under one, never before */ + /* v8 ignore next -- every id the text yields names a verse number */ verseNum: firstVerseNumber(verse) ?? 0, }; } From e9f0aa03f14decbb4dcd7eedfff83f2dac010026 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 13 Oct 2026 09:36:10 -0600 Subject: [PATCH 07/10] Show a deleted verse's free translation as its own card --- contributions/localizedStrings.json | 1 + .../components/AnalysisStore.test.tsx | 8 +- .../components/Interlinearizer.test.tsx | 148 +++++++++++++++++- .../OrphanedFreeTranslationCard.test.tsx | 69 ++++++++ .../components/StaleAnalysesReporter.test.tsx | 71 +++++++-- src/__tests__/hooks/useHydrationRange.test.ts | 2 +- src/__tests__/hooks/useSegmentWindow.test.ts | 23 ++- src/__tests__/utils/hydration-range.test.ts | 2 +- src/__tests__/utils/segment-heights.test.ts | 68 +++++++- .../utils/stale-free-translations.test.ts | 120 +++++++++++--- src/components/AnalysisStore.tsx | 12 +- .../OrphanedFreeTranslationCard.tsx | 57 +++++++ src/components/SegmentListView.tsx | 52 +++++- src/components/StaleAnalysesReporter.tsx | 21 ++- src/hooks/useSegmentHeights.ts | 16 +- src/hooks/useSegmentWindow.ts | 9 +- src/utils/segment-heights.ts | 44 +++++- src/utils/stale-free-translations.ts | 98 +++++++++--- 18 files changed, 724 insertions(+), 97 deletions(-) create mode 100644 src/__tests__/components/OrphanedFreeTranslationCard.test.tsx create mode 100644 src/components/OrphanedFreeTranslationCard.tsx diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 1f0e0d1f2..7af76d51c 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -187,6 +187,7 @@ "%interlinearizer_freeTranslationInput_staleDiscard%": "Discard", "%interlinearizer_freeTranslationInput_staleNoText%": "(no translation in this language)", "%interlinearizer_freeTranslationInput_staleKeepBlocked%": "Clear your translation to keep one of these instead", + "%interlinearizer_freeTranslationInput_orphaned%": "Stale: verse {verse} is no longer in the text", "%interlinearizer_morphemeEditor_splitLabel%": "Split into morphemes", "%interlinearizer_morphemeEditor_reset%": "Reset", "%interlinearizer_morphemeEditor_cancel%": "Cancel", diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index b5e9dfc09..ebe459200 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -29,7 +29,7 @@ import { useSegmentHasApprovedTranslation, useSegmentsWithApprovedTranslation, useStaleFreeTranslationDispatch, - useStaleFreeTranslationsBySegment, + usePlacedStaleFreeTranslations, useStaleTokenLinks, useGloss, useGlossDispatch, @@ -1074,14 +1074,14 @@ describe('useSegmentsWithApprovedTranslation', () => { }); }); -describe('useStaleFreeTranslationsBySegment', () => { +describe('usePlacedStaleFreeTranslations', () => { it("files a stale translation under the book's segment showing it", () => { const book = makeVerseBook([{ sid: 'GEN 1:1', text: 'In the beginning' }]); - const { result } = renderStoreHook(() => useStaleFreeTranslationsBySegment(book), { + const { result } = renderStoreHook(() => usePlacedStaleFreeTranslations(book), { initialAnalysis: withSegmentTranslation('GEN 1:1', 'stale'), }); - expect(result.current).toEqual( + expect(result.current.bySegment).toEqual( new Map([['GEN 1:1', [{ analysisId: 'sa-1', segmentId: 'GEN 1:1', text: 'old' }]]]), ); }); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 6063477ce..5d4a5b871 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -74,6 +74,8 @@ type CapturedSegmentViewProps = { gutterLabel?: string; /** Controls whether tokens are rendered as chips or as raw baseline text. */ displayMode: SegmentDisplayMode; + /** Height a stand-in reserves for the chips it is standing in for. */ + placeholderHeightPx?: number; /** The `Token.ref` string of the currently focused token, if any. */ focusedTokenRef: string | undefined; /** Whether this segment corresponds to the currently active verse. */ @@ -106,9 +108,18 @@ let phraseLinkByIdMapReads = 0; /** What the mocked `useAnalysisReadOnly` reports; reset in `beforeEach`. */ let mockReadOnly = false; -/** Stale free translations served by the mocked `useStaleFreeTranslationsBySegment`. */ +/** + * Stale free translations each segment reviews, served by the mocked + * `usePlacedStaleFreeTranslations`. + */ const mockStaleBySegment = new Map(); +/** Orphaned translations by the segment they follow, served by `usePlacedStaleFreeTranslations`. */ +const mockOrphanedAfter = new Map(); + +/** Orphaned translations ahead of the book, served by `usePlacedStaleFreeTranslations`. */ +const mockOrphanedAhead: StaleFreeTranslation[] = []; + /** Segments the mocked `useSegmentsWithApprovedTranslation` reports holding an approved translation. */ const mockApprovedSegments = new Set(); @@ -117,7 +128,11 @@ jest.mock('../../components/AnalysisStore', () => ({ useAnalysisReadOnly: () => mockReadOnly, /** No segment carries a free translation, so a read-only height table charges none for one. */ useFreeTranslationsBySegment: () => new Map(), - useStaleFreeTranslationsBySegment: () => mockStaleBySegment, + usePlacedStaleFreeTranslations: () => ({ + bySegment: mockStaleBySegment, + orphanedAfter: mockOrphanedAfter, + orphanedAhead: mockOrphanedAhead, + }), useSegmentsWithApprovedTranslation: () => mockApprovedSegments, /** * Pass-through provider stub that renders children directly, keeping AnalysisStore.tsx out of @@ -174,6 +189,16 @@ jest.mock('../../components/ContinuousView', () => ({ }, })); +jest.mock('../../components/OrphanedFreeTranslationCard', () => ({ + __esModule: true, + // The list spreads the real key list into its own, so the stub must still export it. + ORPHANED_CARD_STRING_KEYS: jest.requireActual('../../components/OrphanedFreeTranslationCard') + .ORPHANED_CARD_STRING_KEYS, + default: ({ translation }: Readonly<{ translation: StaleFreeTranslation }>) => ( +
    + ), +})); + jest.mock('../../components/SegmentView', () => ({ __esModule: true, // The list spreads the real key list into its own, so the stub must still export it. @@ -459,6 +484,8 @@ beforeEach(() => { // The phrase-link map is a plain Map (not a jest mock), so resetMocks does not clear it. mockPhraseLinkById.clear(); mockStaleBySegment.clear(); + mockOrphanedAfter.clear(); + mockOrphanedAhead.length = 0; mockApprovedSegments.clear(); capturedSegmentation = undefined; mockReadOnly = false; @@ -1585,6 +1612,123 @@ describe('Interlinearizer', () => { expect(trailingSpacerPx(STALE_BELOW_WINDOW)).toBe(without); }); + it('reserves the cards orphaned after a segment below the window', () => { + const without = trailingSpacerPx(STALE_BELOW_WINDOW); + mockOrphanedAfter.set('GEN 1:190', [ + { analysisId: 'sa-1', segmentId: 'GEN 1:190:9', text: 'A' }, + { analysisId: 'sa-2', segmentId: 'GEN 1:190:9', text: 'B' }, + ]); + + // Each card's gap, chrome, and row. + expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(2 * (8 + 34 + 32)); + }); + + it('reserves the placeholder an orphaned translation with no text in the language shows', () => { + mockKeyAsValueLocalizedStrings({ + '%interlinearizer_freeTranslationInput_staleNoText%': 'a'.repeat(200), + }); + const without = trailingSpacerPx(STALE_BELOW_WINDOW); + mockOrphanedAfter.set('GEN 1:190', [ + { analysisId: 'sa-1', segmentId: 'GEN 1:190:9', text: '' }, + ]); + + expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(8 + 34 + 32 + 2 * 20 + 4); + }); + + it('reserves the cards orphaned ahead of the book above the window', () => { + const leadingSpacerPx = () => { + const { container, unmount } = renderInterlinearizer(STALE_BELOW_WINDOW); + const spacer = container.querySelector('[data-leading-spacer]'); + if (!(spacer instanceof HTMLElement)) throw new Error('leading spacer not found'); + const height = Number.parseFloat(spacer.style.height); + unmount(); + return height; + }; + const without = leadingSpacerPx(); + mockOrphanedAhead.push({ analysisId: 'sa-1', segmentId: 'GEN 1:0', text: 'A' }); + + expect(leadingSpacerPx() - without).toBe(8 + 34 + 32); + }); + + it('reserves no orphaned cards for a read-only analysis', () => { + mockReadOnly = true; + const without = trailingSpacerPx(STALE_BELOW_WINDOW); + mockOrphanedAfter.set('GEN 1:190', [ + { analysisId: 'sa-1', segmentId: 'GEN 1:190:9', text: 'A' }, + ]); + + expect(trailingSpacerPx(STALE_BELOW_WINDOW)).toBe(without); + }); + + it('reserves a stand-in its own box, leaving out the cards orphaned after it', () => { + const standInPx = () => { + capturedSegmentViewPropsList = []; + const { unmount } = renderInterlinearizer(STALE_BELOW_WINDOW); + const props = capturedSegmentViewPropsList.findLast((p) => p.segment.id === 'GEN 1:110'); + unmount(); + return props?.placeholderHeightPx; + }; + const without = standInPx(); + mockOrphanedAfter.set('GEN 1:110', [ + { analysisId: 'sa-1', segmentId: 'GEN 1:110:9', text: 'A' }, + ]); + + expect(without).toEqual(expect.any(Number)); + expect(standInPx()).toBe(without); + }); + + it('shows a translation orphaned after a segment as a card right after it', () => { + mockOrphanedAfter.set('GEN 1:1', [{ analysisId: 'sa-1', segmentId: 'GEN 1:9', text: 'A' }]); + + renderInterlinearizer({ book: makeManySegmentBook(3), showFreeTranslation: true }); + + expect(screen.getByTestId('orphaned-free-translation').previousElementSibling).toHaveAttribute( + 'data-segment-id', + 'GEN 1:1', + ); + }); + + it('shows the translations orphaned ahead of the book in a block before its first segment', () => { + mockOrphanedAhead.push({ analysisId: 'sa-1', segmentId: 'GEN 1:0', text: 'A' }); + + const { container } = renderInterlinearizer({ + book: makeManySegmentBook(3), + showFreeTranslation: true, + }); + + const ahead = container.querySelector('[data-orphaned-ahead]'); + expect(ahead).toContainElement(screen.getByTestId('orphaned-free-translation')); + expect(ahead?.nextElementSibling).toHaveAttribute('data-segment-id', 'GEN 1:1'); + }); + + it('shows no block ahead of the book when nothing is orphaned there', () => { + const { container } = renderInterlinearizer({ + book: makeManySegmentBook(3), + showFreeTranslation: true, + }); + + expect(container.querySelector('[data-orphaned-ahead]')).toBeNull(); + }); + + it('shows no orphaned translations while free translations are hidden', () => { + mockOrphanedAfter.set('GEN 1:1', [{ analysisId: 'sa-1', segmentId: 'GEN 1:9', text: 'A' }]); + mockOrphanedAhead.push({ analysisId: 'sa-2', segmentId: 'GEN 1:0', text: 'A' }); + + renderInterlinearizer({ book: makeManySegmentBook(3), showFreeTranslation: false }); + + expect(screen.queryByTestId('orphaned-free-translation')).toBeNull(); + }); + + it('shows no orphaned translations in a read-only analysis', () => { + mockReadOnly = true; + mockOrphanedAfter.set('GEN 1:1', [{ analysisId: 'sa-1', segmentId: 'GEN 1:9', text: 'A' }]); + mockOrphanedAhead.push({ analysisId: 'sa-2', segmentId: 'GEN 1:0', text: 'A' }); + + renderInterlinearizer({ book: makeManySegmentBook(3), showFreeTranslation: true }); + + expect(screen.queryByTestId('orphaned-free-translation')).toBeNull(); + }); + it('reserves nothing above a window that starts at the first segment', () => { const book = makeManySegmentBook(40); const { container } = renderInterlinearizer({ diff --git a/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx new file mode 100644 index 000000000..9251980b4 --- /dev/null +++ b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx @@ -0,0 +1,69 @@ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import OrphanedFreeTranslationCard from '../../components/OrphanedFreeTranslationCard'; +import type { StaleFreeTranslation } from '../../utils/stale-free-translations'; + +const mockDiscard = jest.fn(); + +jest.mock('../../components/AnalysisStore', () => ({ + __esModule: true, + useStaleFreeTranslationDispatch: () => ({ keep: jest.fn(), discard: mockDiscard }), +})); + +jest.mock('../../utils/stale-free-translations', () => ({ + __esModule: true, + verseLabelOf: (segmentId: string) => `verse of ${segmentId}`, +})); + +const STRINGS = { + '%interlinearizer_freeTranslationInput_orphaned%': 'Gone: {verse}', + '%interlinearizer_freeTranslationInput_staleDiscard%': 'Discard', + '%interlinearizer_freeTranslationInput_staleNoText%': '(none)', +}; + +/** Renders the card for a translation of a piece of GEN 1:5 split off mid-verse, reading `text`. */ +function renderCard(text = 'Au début') { + const translation: StaleFreeTranslation = { analysisId: 'sa-1', segmentId: 'GEN 1:5:12', text }; + return render( + , + ); +} + +describe('OrphanedFreeTranslationCard', () => { + it('names the verse the translation was written for', () => { + renderCard(); + + expect(screen.getByTestId('orphaned-free-translation')).toHaveTextContent( + 'Gone: verse of GEN 1:5:12', + ); + }); + + it("shows the translation's text", () => { + renderCard(); + + expect(screen.getByTestId('orphaned-free-translation')).toHaveTextContent('Au début'); + }); + + it('shows the placeholder for a translation with no text in the language', () => { + renderCard(''); + + expect(screen.getByTestId('orphaned-free-translation')).toHaveTextContent('(none)'); + }); + + it('discards the translation', async () => { + renderCard(); + + await userEvent.click(screen.getByTestId('orphaned-free-translation-discard')); + + expect(mockDiscard).toHaveBeenCalledWith('sa-1'); + }); + + it('offers nothing to do with the translation but discard it', () => { + renderCard(); + + expect(screen.getAllByRole('button')).toHaveLength(1); + }); +}); diff --git a/src/__tests__/components/StaleAnalysesReporter.test.tsx b/src/__tests__/components/StaleAnalysesReporter.test.tsx index 63a1d66b9..8fe3d5616 100644 --- a/src/__tests__/components/StaleAnalysesReporter.test.tsx +++ b/src/__tests__/components/StaleAnalysesReporter.test.tsx @@ -2,18 +2,18 @@ import { render } from '@testing-library/react'; import type { TokenAnalysisLink } from 'interlinearizer'; -import { - useStaleFreeTranslationsBySegment, - useStaleTokenLinks, -} from '../../components/AnalysisStore'; +import { usePlacedStaleFreeTranslations, useStaleTokenLinks } from '../../components/AnalysisStore'; import StaleAnalysesReporter from '../../components/StaleAnalysesReporter'; -import type { StaleFreeTranslation } from '../../utils/stale-free-translations'; -import { FIXTURE_STAMPS, GEN_1_1_BOOK } from '../test-helpers'; +import type { + PlacedStaleFreeTranslations, + StaleFreeTranslation, +} from '../../utils/stale-free-translations'; +import { FIXTURE_STAMPS, GEN_1_1_BOOK, makeVerseBook } from '../test-helpers'; jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, useStaleTokenLinks: jest.fn(), - useStaleFreeTranslationsBySegment: jest.fn(), + usePlacedStaleFreeTranslations: jest.fn(), })); function staleLink(analysisId: string, tokenRef: string): TokenAnalysisLink { @@ -24,15 +24,28 @@ function staleTranslation(analysisId: string, segmentId: string): StaleFreeTrans return { analysisId, segmentId, text: 'Au début' }; } -/** Serves the store's stale analyses: these stale links, and these translations by segment. */ +/** Serves the store's stale analyses: these stale links, and these translations as placed. */ function mockStale( links: readonly TokenAnalysisLink[], translationsBySegment: ReadonlyMap = new Map(), + orphaned: Partial = {}, ): void { jest.mocked(useStaleTokenLinks).mockReturnValue(links); - jest.mocked(useStaleFreeTranslationsBySegment).mockReturnValue(translationsBySegment); + jest.mocked(usePlacedStaleFreeTranslations).mockReturnValue({ + bySegment: translationsBySegment, + orphanedAfter: new Map(), + orphanedAhead: [], + ...orphaned, + }); } +/** Verses 1 through 3 of Genesis 1, each its own segment. */ +const THREE_VERSE_BOOK = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:2', text: 'and the earth' }, + { sid: 'GEN 1:3', text: 'and God said' }, +]); + describe('StaleAnalysesReporter', () => { it('reports a key per place in the book an analysis went stale at', () => { mockStale([staleLink('a', 'GEN 1:1:0'), staleLink('b', 'GEN 1:1:0')]); @@ -72,13 +85,13 @@ describe('StaleAnalysesReporter', () => { mockStale( [], new Map([ - ['GEN 1:1', [staleTranslation('s1', 'GEN 1:1'), staleTranslation('s2', 'GEN 1:1:9')]], ['GEN 1:2', [staleTranslation('s3', 'GEN 1:2')]], + ['GEN 1:1', [staleTranslation('s1', 'GEN 1:1'), staleTranslation('s2', 'GEN 1:1:9')]], ]), ); const onReport = jest.fn(); - render(); + render(); expect(onReport).toHaveBeenLastCalledWith( expect.objectContaining({ @@ -91,6 +104,42 @@ describe('StaleAnalysesReporter', () => { ); }); + it('reports a translation orphaned after a segment with that segment, after its own', () => { + mockStale([], new Map([['GEN 1:1', [staleTranslation('s1', 'GEN 1:1')]]]), { + orphanedAfter: new Map([['GEN 1:1', [staleTranslation('s2', 'GEN 1:9')]]]), + }); + const onReport = jest.fn(); + + render(); + + expect(onReport).toHaveBeenLastCalledWith( + expect.objectContaining({ + freeTranslations: [ + { key: 'freeTranslation s1', segmentId: 'GEN 1:1' }, + { key: 'freeTranslation s2', segmentId: 'GEN 1:1' }, + ], + }), + ); + }); + + it("reports a translation orphaned ahead of the book with its first segment, before that segment's own", () => { + mockStale([], new Map([['GEN 1:1', [staleTranslation('s1', 'GEN 1:1')]]]), { + orphanedAhead: [staleTranslation('s0', 'GEN 1:0')], + }); + const onReport = jest.fn(); + + render(); + + expect(onReport).toHaveBeenLastCalledWith( + expect.objectContaining({ + freeTranslations: [ + { key: 'freeTranslation s0', segmentId: 'GEN 1:1' }, + { key: 'freeTranslation s1', segmentId: 'GEN 1:1' }, + ], + }), + ); + }); + it('reports again when the stale analyses change', () => { mockStale([staleLink('a', 'GEN 1:1:0')]); const onReport = jest.fn(); diff --git a/src/__tests__/hooks/useHydrationRange.test.ts b/src/__tests__/hooks/useHydrationRange.test.ts index 99ea794fb..e3a7d9459 100644 --- a/src/__tests__/hooks/useHydrationRange.test.ts +++ b/src/__tests__/hooks/useHydrationRange.test.ts @@ -16,7 +16,7 @@ function uniformTable(count: number): HeightTable { const heights = Array.from({ length: count }, () => SEGMENT_PX); const offsets = heights.map((_h, i) => i * SEGMENT_PX); offsets.push(count * SEGMENT_PX); - return { heights, offsets, total: count * SEGMENT_PX }; + return { heights, boxHeights: heights, offsets, total: count * SEGMENT_PX }; } /** diff --git a/src/__tests__/hooks/useSegmentWindow.test.ts b/src/__tests__/hooks/useSegmentWindow.test.ts index 8f3213120..91f846336 100644 --- a/src/__tests__/hooks/useSegmentWindow.test.ts +++ b/src/__tests__/hooks/useSegmentWindow.test.ts @@ -32,7 +32,7 @@ function uniformHeightTable(book: Book): HeightTable { const heights = book.segments.map(() => UNIFORM_SEGMENT_PX); const offsets = heights.map((_h, i) => i * UNIFORM_SEGMENT_PX); offsets.push(heights.length * UNIFORM_SEGMENT_PX); - return { heights, offsets, total: heights.length * UNIFORM_SEGMENT_PX }; + return { heights, boxHeights: heights, offsets, total: heights.length * UNIFORM_SEGMENT_PX }; } /** One animation frame under fake timers, which schedules `requestAnimationFrame` on a 16ms clock. */ @@ -1962,6 +1962,27 @@ describe('useSegmentWindow', () => { expect(scrollIntoView).toHaveBeenCalledWith({ behavior: 'auto', block: 'start' }); }); + it('snaps the cards ahead of the first segment to the top with it', () => { + const book = makeBook(60, 0); + const { container, rerender } = renderSegmentWindow(book, { + book: 'GEN', + chapterNum: 1, + verseNum: 1, + }); + const ahead = document.createElement('div'); + ahead.setAttribute('data-orphaned-ahead', ''); + ahead.scrollIntoView = jest.fn(); + container.appendChild(ahead); + const active = mountActiveSegment(container); + active.scrollIntoView = jest.fn(); + + act(() => rerender({ b: book, ref: { book: 'GEN', chapterNum: 1, verseNum: 50 } })); + act(() => jest.advanceTimersByTime(RECENTER_FADE_MS)); + + expect(ahead.scrollIntoView).toHaveBeenCalledWith({ behavior: 'auto', block: 'start' }); + expect(active.scrollIntoView).not.toHaveBeenCalled(); + }); + it('grows the snap spacer when scrollIntoView cannot reach the top', () => { const book = makeBook(60, 0); const scrollIntoView = jest.fn(); diff --git a/src/__tests__/utils/hydration-range.test.ts b/src/__tests__/utils/hydration-range.test.ts index b3ed9d405..02f7f6ad2 100644 --- a/src/__tests__/utils/hydration-range.test.ts +++ b/src/__tests__/utils/hydration-range.test.ts @@ -22,7 +22,7 @@ function uniformTable(count: number): HeightTable { const heights = Array.from({ length: count }, () => SEGMENT_PX); const offsets = heights.map((_h, i) => i * SEGMENT_PX); offsets.push(count * SEGMENT_PX); - return { heights, offsets, total: count * SEGMENT_PX }; + return { heights, boxHeights: heights, offsets, total: count * SEGMENT_PX }; } describe('hydrationTarget', () => { diff --git a/src/__tests__/utils/segment-heights.test.ts b/src/__tests__/utils/segment-heights.test.ts index 111cdfe3a..9911a519f 100644 --- a/src/__tests__/utils/segment-heights.test.ts +++ b/src/__tests__/utils/segment-heights.test.ts @@ -6,6 +6,7 @@ import { heightForRows, offsetOfSegment, predictLineCount, + predictOrphanedCardHeights, predictRowCount, predictSegmentHeights, segmentIndexAtOffset, @@ -286,7 +287,12 @@ describe('buildHeightTable', () => { }); it('builds an empty table for a book with no segments', () => { - expect(buildHeightTable(CONFIG, [])).toEqual({ heights: [], offsets: [0], total: 0 }); + expect(buildHeightTable(CONFIG, [])).toEqual({ + heights: [], + boxHeights: [], + offsets: [0], + total: 0, + }); }); it('adds the gap between segments to each height after the first', () => { @@ -307,12 +313,72 @@ describe('buildHeightTable', () => { expect(table.heights[0]).toBe(100); }); + it('adds the cards beside a segment to its height', () => { + const table = buildHeightTable({ ...CONFIG, segmentGapPx: 8 }, [100, 100], [74, 0]); + expect(table.heights).toEqual([174, 108]); + }); + + it("reports each segment's own box apart from its gap and the cards beside it", () => { + const table = buildHeightTable({ ...CONFIG, segmentGapPx: 8 }, [100, 100], [74, 74]); + expect(table.boxHeights).toEqual([100, 100]); + }); + it('reports a total that spans every gap between segments', () => { const table = buildHeightTable({ ...CONFIG, segmentGapPx: 8 }, [100, 100, 100]); expect(table.total).toBe(316); }); }); +describe('predictOrphanedCardHeights', () => { + const withCards = { ...CONFIG, showFreeTranslation: true, segmentGapPx: 8 }; + + it('charges nothing beside a segment showing no cards', () => { + expect(predictOrphanedCardHeights(2, withCards, 300)).toEqual([0, 0]); + }); + + it('charges each card its chrome, its row, and the gap beside it', () => { + const config = { ...withCards, orphanedTranslationTexts: () => ['A', 'B'] }; + + expect(predictOrphanedCardHeights(1, config, 300)).toEqual([2 * (8 + 34 + 32)]); + }); + + it('charges no gap beside a card where the config sets none', () => { + const config = { + ...CONFIG, + showFreeTranslation: true, + orphanedTranslationTexts: () => ['A'], + }; + + expect(predictOrphanedCardHeights(1, config, 300)).toEqual([34 + 32]); + }); + + it('charges only the segments showing cards', () => { + const config = { + ...withCards, + orphanedTranslationTexts: (index: number) => (index === 1 ? ['A'] : []), + }; + + expect(predictOrphanedCardHeights(2, config, 300)).toEqual([0, 8 + 34 + 32]); + }); + + it('charges the wrapped lines of a card too long to share a line with its button', () => { + const config = { ...withCards, orphanedTranslationTexts: () => ['a'.repeat(200)] }; + + // 200 characters at 6.2px wrap onto five 300px lines, the button on a line below them. + expect(predictOrphanedCardHeights(1, config, 300)).toEqual([8 + 34 + 32 + 5 * 20 + 4]); + }); + + it('charges nothing while free translations are hidden', () => { + const config = { + ...withCards, + showFreeTranslation: false, + orphanedTranslationTexts: () => ['A'], + }; + + expect(predictOrphanedCardHeights(1, config, 300)).toEqual([0]); + }); +}); + describe('segmentIndexAtOffset', () => { function table() { return buildHeightTable(CONFIG, [132, 132, 132]); diff --git a/src/__tests__/utils/stale-free-translations.test.ts b/src/__tests__/utils/stale-free-translations.test.ts index 9b1b33cf6..cb4c6445e 100644 --- a/src/__tests__/utils/stale-free-translations.test.ts +++ b/src/__tests__/utils/stale-free-translations.test.ts @@ -6,6 +6,7 @@ import { nextSegmentAmong, placeStaleFreeTranslations, type StaleFreeTranslation, + verseLabelOf, } from '../../utils/stale-free-translations'; import { makeVerseBook } from '../test-helpers'; @@ -21,17 +22,34 @@ const verseBook = makeVerseBook([ { sid: 'GEN 1:3', text: 'and God said' }, ]); -/** The analysis ids filed under each segment, keyed by segment id. */ +/** The analysis ids in each list, keyed by segment id. */ +function idsBySegment( + bySegment: ReadonlyMap, +): Record { + return Object.fromEntries( + [...bySegment].map(([segmentId, list]) => [segmentId, list.map((t) => t.analysisId)]), + ); +} + +/** The analysis ids filed under each segment for its own review, keyed by segment id. */ function placed( translations: readonly StaleFreeTranslation[], book = verseBook, ): Record { - return Object.fromEntries( - [...placeStaleFreeTranslations(translations, book)].map(([segmentId, list]) => [ - segmentId, - list.map((t) => t.analysisId), - ]), - ); + return idsBySegment(placeStaleFreeTranslations(translations, book).bySegment); +} + +/** The analysis ids orphaned after each segment, keyed by segment id. */ +function orphanedAfter( + translations: readonly StaleFreeTranslation[], + book = verseBook, +): Record { + return idsBySegment(placeStaleFreeTranslations(translations, book).orphanedAfter); +} + +/** The analysis ids orphaned ahead of the book's first segment. */ +function orphanedAhead(translations: readonly StaleFreeTranslation[], book = verseBook): string[] { + return placeStaleFreeTranslations(translations, book).orphanedAhead.map((t) => t.analysisId); } describe('placeStaleFreeTranslations', () => { @@ -109,7 +127,7 @@ describe('placeStaleFreeTranslations', () => { expect(placed([stale('GEN 1:1/s1')], book)).toEqual({ 'GEN 1:1': ['sa GEN 1:1/s1'] }); }); - it('files a translation of a deleted verse under the segment ending the verse before it', () => { + it('orphans a translation of a deleted verse after the segment ending the verse before it', () => { const book = resegmentBook( makeVerseBook([ { sid: 'GEN 1:1', text: 'In the beginning God created' }, @@ -118,45 +136,89 @@ describe('placeStaleFreeTranslations', () => { { removedVerseStarts: [], addedStarts: [{ tokenRef: 'GEN 1:1:17', surfaceText: 'God' }] }, ); - expect(placed([stale('GEN 1:2')], book)).toEqual({ 'GEN 1:1:17': ['sa GEN 1:2'] }); + expect(orphanedAfter([stale('GEN 1:2')], book)).toEqual({ 'GEN 1:1:17': ['sa GEN 1:2'] }); }); - it('files a translation of a deleted verse under the last segment of an earlier chapter', () => { + it('files a translation of a deleted verse under no segment for review', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:3', text: 'and God said' }, + ]); + + expect(placed([stale('GEN 1:2')], book)).toEqual({}); + }); + + it('orphans a translation of a deleted verse after the last segment of an earlier chapter', () => { const book = makeVerseBook([ { sid: 'GEN 1:1', text: 'In the beginning' }, { sid: 'GEN 1:3', text: 'and God said' }, { sid: 'GEN 2:1', text: 'Thus the heavens' }, ]); - expect(placed([stale('GEN 1:9')], book)).toEqual({ 'GEN 1:3': ['sa GEN 1:9'] }); + expect(orphanedAfter([stale('GEN 1:9')], book)).toEqual({ 'GEN 1:3': ['sa GEN 1:9'] }); }); - it('files a translation of a deleted verse ahead of every other under the first segment', () => { + it('orphans a translation of a deleted verse ahead of every other ahead of the book', () => { const book = makeVerseBook([ { sid: 'GEN 1:2', text: 'and the earth was void' }, { sid: 'GEN 1:3', text: 'and God said' }, ]); - expect(placed([stale('GEN 1:1')], book)).toEqual({ 'GEN 1:2': ['sa GEN 1:1'] }); + expect(orphanedAhead([stale('GEN 1:1')], book)).toEqual(['sa GEN 1:1']); + }); + + it('orphans nothing ahead of the book when an earlier verse remains', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:3', text: 'and God said' }, + ]); + + expect(orphanedAhead([stale('GEN 1:2')], book)).toEqual([]); }); - it('files a translation of a heading whose verse was deleted under the segment before it', () => { - expect(placed([stale('GEN 1:9/s1')])).toEqual({ 'GEN 1:3': ['sa GEN 1:9/s1'] }); + it('orphans a translation of a heading whose verse was deleted after the segment before it', () => { + expect(orphanedAfter([stale('GEN 1:9/s1')])).toEqual({ 'GEN 1:3': ['sa GEN 1:9/s1'] }); }); - it("files a deleted verse's translation after the translations of the segment it falls under", () => { + it('lists the translations orphaned after a segment in the order their verses ran', () => { const book = makeVerseBook([ { sid: 'GEN 1:1', text: 'In the beginning' }, - { sid: 'GEN 1:3', text: 'and God said' }, + { sid: 'GEN 2:1', text: 'Thus the heavens' }, ]); - expect(placed([stale('GEN 1:2'), stale('GEN 1:1')], book)).toEqual({ - 'GEN 1:1': ['sa GEN 1:1', 'sa GEN 1:2'], + expect(orphanedAfter([stale('GEN 1:3'), stale('GEN 1:2')], book)).toEqual({ + 'GEN 1:1': ['sa GEN 1:2', 'sa GEN 1:3'], }); }); + it('lists the translations orphaned in one chapter ahead of those in a later one', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 3:1', text: 'Now the serpent' }, + ]); + + expect(orphanedAfter([stale('GEN 2:1'), stale('GEN 1:5')], book)).toEqual({ + 'GEN 1:1': ['sa GEN 1:5', 'sa GEN 2:1'], + }); + }); + + it('lists the translations orphaned ahead of the book in the order their verses ran', () => { + const book = makeVerseBook([{ sid: 'GEN 1:3', text: 'and God said' }]); + + expect(orphanedAhead([stale('GEN 1:2'), stale('GEN 1:1')], book)).toEqual([ + 'sa GEN 1:1', + 'sa GEN 1:2', + ]); + }); + it('shows a translation nowhere in a book holding no text', () => { - expect(placed([stale('GEN 1:1')], makeVerseBook([]))).toEqual({}); + const placedNowhere = placeStaleFreeTranslations([stale('GEN 1:1')], makeVerseBook([])); + + expect(placedNowhere).toEqual({ + bySegment: new Map(), + orphanedAfter: new Map(), + orphanedAhead: [], + }); }); it('shows a translation of another book nowhere', () => { @@ -164,6 +226,24 @@ describe('placeStaleFreeTranslations', () => { }); }); +describe('verseLabelOf', () => { + it('names the verse a segment begun at a verse falls in', () => { + expect(verseLabelOf('GEN 1:5')).toBe('1:5'); + }); + + it('names a verse range verbatim', () => { + expect(verseLabelOf('GEN 1:5-6')).toBe('1:5-6'); + }); + + it('names the verse a segment begun mid-verse falls in', () => { + expect(verseLabelOf('GEN 1:5:12')).toBe('1:5'); + }); + + it('names the verse a heading is filed under', () => { + expect(verseLabelOf('GEN 1:5/s1#2')).toBe('1:5'); + }); +}); + describe('adoptedStaleTranslation', () => { it('adopts the only stale translation of a segment holding no approved one', () => { const only = stale('GEN 1:1'); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 2ddc9de34..3797b0265 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -71,7 +71,7 @@ import { emptyAnalysis } from '../types/empty-factories'; import type { CatalogRow, HeadingPlacement } from '../utils/analysis-query'; import { placeStaleFreeTranslations, - type StaleFreeTranslation, + type PlacedStaleFreeTranslations, } from '../utils/stale-free-translations'; import { resolvedTokenAnalysisEqual, type ResolvedTokenAnalysis } from '../utils/suggestion-engine'; @@ -1222,16 +1222,12 @@ export function useSegmentsWithApprovedTranslation(): ReadonlySet { } /** - * Returns the stale free translations each of `book`'s segments shows, keyed by segment id, for the - * segments showing any. A translation whose own segment has vanished is shown by the segment now - * covering where it began. + * Returns `book`'s stale free translations, filed by where the view shows each. * * @throws When called outside an {@link AnalysisStoreProvider}. */ -export function useStaleFreeTranslationsBySegment( - book: Book, -): ReadonlyMap { - useRequiredCallbacks('useStaleFreeTranslationsBySegment'); +export function usePlacedStaleFreeTranslations(book: Book): PlacedStaleFreeTranslations { + useRequiredCallbacks('usePlacedStaleFreeTranslations'); const stale = useSelector((state: AnalysisRootState) => selectStaleFreeTranslations(state.analysis), diff --git a/src/components/OrphanedFreeTranslationCard.tsx b/src/components/OrphanedFreeTranslationCard.tsx new file mode 100644 index 000000000..d0bb70c6c --- /dev/null +++ b/src/components/OrphanedFreeTranslationCard.tsx @@ -0,0 +1,57 @@ +import { Button } from 'platform-bible-react'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; +import { useStaleFreeTranslationDispatch } from './AnalysisStore'; +import { verseLabelOf, type StaleFreeTranslation } from '../utils/stale-free-translations'; + +/** Localized string keys {@link OrphanedFreeTranslationCard} reads from the strings it is handed. */ +export const ORPHANED_CARD_STRING_KEYS = [ + '%interlinearizer_freeTranslationInput_orphaned%', + '%interlinearizer_freeTranslationInput_staleDiscard%', + '%interlinearizer_freeTranslationInput_staleNoText%', +] as const satisfies `%${string}%`[]; + +type OrphanedFreeTranslationCardProps = Readonly<{ + translation: StaleFreeTranslation; + /** Resolved {@link ORPHANED_CARD_STRING_KEYS}, supplied rather than subscribed to per card. */ + localizedStrings: LanguageStrings; +}>; + +/** + * A stale free translation of a verse the book no longer holds, shown as an item of its own in the + * segment list. It offers only Discard, since no segment remains to keep it for. + */ +export default function OrphanedFreeTranslationCard({ + translation, + localizedStrings, +}: OrphanedFreeTranslationCardProps) { + const { discard } = useStaleFreeTranslationDispatch(); + return ( +
    + + {formatReplacementString( + localizedStrings['%interlinearizer_freeTranslationInput_orphaned%'], + { verse: verseLabelOf(translation.segmentId) }, + )} + +
    + + {translation.text || + localizedStrings['%interlinearizer_freeTranslationInput_staleNoText%']} + + +
    +
    + ); +} diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 4dd72ef66..8e46b7e4b 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -17,17 +17,23 @@ import type { ViewOptions } from '../types/view-options'; import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { altHeldSwap } from './alt-key-hint'; import { buildSegmentLabels } from '../utils/segment-labels'; -import { adoptedStaleTranslation } from '../utils/stale-free-translations'; +import { + adoptedStaleTranslation, + type StaleFreeTranslation, +} from '../utils/stale-free-translations'; import { segmentContainsVerse } from '../utils/verse-ref'; import { buildVerseStartLabels } from '../utils/verse-superscripts'; import { useAnalysisReadOnly, useFreeTranslationsBySegment, useSegmentsWithApprovedTranslation, - useStaleFreeTranslationsBySegment, + usePlacedStaleFreeTranslations, } from './AnalysisStore'; import { useFocus, useFocusActions } from './FocusStore'; import type { RecenterRequest } from './InterlinearNavContext'; +import OrphanedFreeTranslationCard, { + ORPHANED_CARD_STRING_KEYS, +} from './OrphanedFreeTranslationCard'; import { useSegmentation } from './SegmentationStore'; import MemoizedSegmentView, { SEGMENT_STRING_KEYS, type SegmentDisplayMode } from './SegmentView'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; @@ -97,7 +103,7 @@ const MERGE_CONTROL_GAP_PX = 24; const LIST_STRING_KEYS = [ '%interlinearizer_segmentList_scrollToActiveVerse%', '%interlinearizer_segmentList_noVerseData%', - '%interlinearizer_freeTranslationInput_staleNoText%', + ...ORPHANED_CARD_STRING_KEYS, ...SEGMENT_STRING_KEYS, ] as const satisfies `%${string}%`[]; @@ -325,6 +331,9 @@ export default function SegmentListView({ scrollContainerRef.current = el ?? undefined; }, []); + /** Whether translations of verses the book no longer holds show, as cards of their own. */ + const showsOrphanedCards = viewOptions.showFreeTranslation && !readOnly; + /** Whether the current state offers merge controls at all, before per-segment eligibility. */ const showsMergeControls = phraseMode.kind === 'view' && !readOnly; @@ -336,7 +345,11 @@ export default function SegmentListView({ ); const freeTranslationsBySegment = useFreeTranslationsBySegment(); - const staleFreeTranslationsBySegment = useStaleFreeTranslationsBySegment(book); + const { + bySegment: staleFreeTranslationsBySegment, + orphanedAfter, + orphanedAhead, + } = usePlacedStaleFreeTranslations(book); const segmentsWithApprovedTranslation = useSegmentsWithApprovedTranslation(); /** @@ -367,6 +380,16 @@ export default function SegmentListView({ [staleFreeTranslationsBySegment, segmentsWithApprovedTranslation, book.segments, staleNoText], ); + /** The text of each orphaned-translation card rendered beside a segment, ahead of it or after. */ + const orphanedTranslationTexts = useCallback( + (index: number) => + [ + ...(index === 0 ? orphanedAhead : []), + ...(orphanedAfter.get(book.segments[index].id) ?? []), + ].map((translation) => translation.text || staleNoText), + [orphanedAhead, orphanedAfter, book.segments, staleNoText], + ); + /** Whether a segment's review rows offer Keep, which they do only while it has no approval. */ const staleReviewOffersKeep = useCallback( (index: number) => !segmentsWithApprovedTranslation.has(book.segments[index].id), @@ -400,6 +423,7 @@ export default function SegmentListView({ freeTranslationText: readOnly ? freeTranslationText : undefined, staleReviewTexts: readOnly ? undefined : staleReviewTexts, staleReviewOffersKeep, + orphanedTranslationTexts: readOnly ? undefined : orphanedTranslationTexts, segmentGapPx: SEGMENT_ROW_GAP_PX, extraGapPx, }), @@ -411,6 +435,7 @@ export default function SegmentListView({ freeTranslationText, staleReviewTexts, staleReviewOffersKeep, + orphanedTranslationTexts, extraGapPx, ], ); @@ -556,6 +581,15 @@ export default function SegmentListView({ }; }, [scrollContainerRef, chapterByIndexRef, heightTableRef, rangeRef]); + const orphanedCards = (translations: readonly StaleFreeTranslation[] | undefined) => + translations?.map((translation) => ( + + )); + return (
    {/* Chapter header band — a real row above the scroll area, not an overlay inside it, so @@ -647,13 +681,18 @@ export default function SegmentListView({ const displayMode = segmentDisplayMode(bookIndex); // A stand-in holds the height its chips will occupy so hydrating shifts nothing below // it; a segment rendering as baseline text in its own right takes its natural height. - // The reserved height is the segment's own box, so its gap comes back off. const placeholderHeightPx = displayMode === 'baseline-text' && !displayContinuousScroll - ? heightTable.heights[bookIndex] - gapAbovePx(bookIndex) + ? heightTable.boxHeights[bookIndex] : undefined; return ( + {/* Tagged so a recenter onto the first segment frames these with it. */} + {showsOrphanedCards && bookIndex === 0 && orphanedAhead.length > 0 && ( +
    + {orphanedCards(orphanedAhead)} +
    + )} {showMergeControl && ( )} @@ -681,6 +720,7 @@ export default function SegmentListView({ viewOptions={viewOptions} staleFreeTranslations={staleFreeTranslationsBySegment.get(seg.id)} /> + {showsOrphanedCards && orphanedCards(orphanedAfter.get(seg.id))}     ); })} diff --git a/src/components/StaleAnalysesReporter.tsx b/src/components/StaleAnalysesReporter.tsx index b22adae2c..0a8f0a1ba 100644 --- a/src/components/StaleAnalysesReporter.tsx +++ b/src/components/StaleAnalysesReporter.tsx @@ -1,7 +1,7 @@ import type { Book } from 'interlinearizer'; import { useEffect, useMemo, useRef } from 'react'; import { bookOfRef } from '../utils/analysis-book'; -import { useStaleFreeTranslationsBySegment, useStaleTokenLinks } from './AnalysisStore'; +import { usePlacedStaleFreeTranslations, useStaleTokenLinks } from './AnalysisStore'; /** The stale analyses a loaded book holds, each named by a key that holds while it stays stale. */ export type StaleAnalysesReport = Readonly<{ @@ -9,8 +9,8 @@ export type StaleAnalysesReport = Readonly<{ /** A key per place in the book an analysis went stale at. */ glosses: readonly string[]; /** - * A key per stale free translation the book shows, with the segment showing it, in document - * order. + * A key per stale free translation the book shows, with the segment showing it or, for one whose + * verse is gone, the segment it is shown beside, in document order. */ freeTranslations: readonly Readonly<{ key: string; segmentId: string }>[]; }>; @@ -41,7 +41,7 @@ export default function StaleAnalysesReporter({ onReport: (report: StaleAnalysesReport | undefined) => void; }>) { const staleTokenLinks = useStaleTokenLinks(); - const freeTranslationsBySegment = useStaleFreeTranslationsBySegment(book); + const placedFreeTranslations = usePlacedStaleFreeTranslations(book); const report = useMemo( () => ({ @@ -53,11 +53,16 @@ export default function StaleAnalysesReporter({ .map((link) => `gloss ${link.analysisId} ${link.token.tokenRef}`), ), ], - freeTranslations: [...freeTranslationsBySegment].flatMap(([segmentId, translations]) => - translations.map((t) => ({ key: `freeTranslation ${t.analysisId}`, segmentId })), - ), + freeTranslations: book.segments.flatMap(({ id: segmentId }, index) => { + const { bySegment, orphanedAfter, orphanedAhead } = placedFreeTranslations; + return [ + ...(index === 0 ? orphanedAhead : []), + ...(bySegment.get(segmentId) ?? []), + ...(orphanedAfter.get(segmentId) ?? []), + ].map((t) => ({ key: `freeTranslation ${t.analysisId}`, segmentId })); + }), }), - [book.bookRef, staleTokenLinks, freeTranslationsBySegment], + [book.bookRef, book.segments, staleTokenLinks, placedFreeTranslations], ); // Every analysis edit rebuilds the stale links, and reporting each rebuild would re-render the diff --git a/src/hooks/useSegmentHeights.ts b/src/hooks/useSegmentHeights.ts index 769dee604..a540ba93a 100644 --- a/src/hooks/useSegmentHeights.ts +++ b/src/hooks/useSegmentHeights.ts @@ -2,7 +2,11 @@ import type { Book } from 'interlinearizer'; import type { RefObject } from 'react'; import { useEffect, useMemo, useState } from 'react'; import type { HeightConfig, HeightTable } from '../utils/segment-heights'; -import { buildHeightTable, predictSegmentHeights } from '../utils/segment-heights'; +import { + buildHeightTable, + predictOrphanedCardHeights, + predictSegmentHeights, +} from '../utils/segment-heights'; /** Width used for the wrap box before the container has been laid out. */ const FALLBACK_WRAP_WIDTH_PX = 1000; @@ -60,5 +64,13 @@ export default function useSegmentHeights({ [book.segments, config, wrapWidth], ); - return useMemo(() => buildHeightTable(config, predicted), [config, predicted]); + const cardHeights = useMemo( + () => predictOrphanedCardHeights(book.segments.length, config, wrapWidth), + [book.segments.length, config, wrapWidth], + ); + + return useMemo( + () => buildHeightTable(config, predicted, cardHeights), + [config, predicted, cardHeights], + ); } diff --git a/src/hooks/useSegmentWindow.ts b/src/hooks/useSegmentWindow.ts index e18143647..790f46ff0 100644 --- a/src/hooks/useSegmentWindow.ts +++ b/src/hooks/useSegmentWindow.ts @@ -584,14 +584,17 @@ export default function useSegmentWindow({ ); /* v8 ignore next -- the recentered target is always mounted, so its element exists */ if (!target) return; + // Cards the list renders ahead of the book's first segment frame with it. + const ahead = target.previousElementSibling; + const frame = ahead?.hasAttribute('data-orphaned-ahead') ? ahead : target; const spacer = container.querySelector('[data-snap-spacer]'); if (spacer) spacer.style.height = '0px'; - target.scrollIntoView({ behavior: 'auto', block: 'start' }); + frame.scrollIntoView({ behavior: 'auto', block: 'start' }); const remainingOffset = - target.getBoundingClientRect().top - container.getBoundingClientRect().top; + frame.getBoundingClientRect().top - container.getBoundingClientRect().top; if (remainingOffset > 1 && spacer) { spacer.style.height = `${Math.ceil(remainingOffset)}px`; - target.scrollIntoView({ behavior: 'auto', block: 'start' }); + frame.scrollIntoView({ behavior: 'auto', block: 'start' }); } }, [scrollContainerRef]); diff --git a/src/utils/segment-heights.ts b/src/utils/segment-heights.ts index 861eaefaf..8a6c9efe5 100644 --- a/src/utils/segment-heights.ts +++ b/src/utils/segment-heights.ts @@ -51,6 +51,9 @@ const STALE_REVIEW_GAP_PX = 4; /** Height of one line of stale translation text listed for review. */ const STALE_REVIEW_TEXT_LINE_PX = 20; +/** Orphaned-translation card chrome above its row: its border, padding, and heading line. */ +const ORPHANED_CARD_BASE_PX = 34; + /** Width assumed for one character of plain baseline text, in pixels. */ const BASELINE_CHAR_PX = 8.4; @@ -82,6 +85,11 @@ export type HeightConfig = Readonly<{ * charging both buttons. */ staleReviewOffersKeep?: (index: number) => boolean; + /** + * The text of each card the list renders beside the segment at `index` for a stale translation of + * a verse the book no longer holds. Defaults to none. + */ + orphanedTranslationTexts?: (index: number) => readonly string[]; /** Which renderer the segment uses; `baseline-text` has no chips and so no rows. */ displayMode: 'token-chip' | 'baseline-text'; /** @@ -211,13 +219,40 @@ export function predictSegmentHeights( }); } +/** + * Predicts the height of the orphaned-translation cards the list renders beside each segment, a gap + * apiece included. The result is index-aligned with a book of `segmentCount` segments. + */ +export function predictOrphanedCardHeights( + segmentCount: number, + config: HeightConfig, + wrapWidth: number, +): readonly number[] { + return Array.from({ length: segmentCount }, (_unused, index) => { + if (!config.showFreeTranslation) return 0; + return (config.orphanedTranslationTexts?.(index) ?? []).reduce( + (height, text) => + height + + (config.segmentGapPx ?? 0) + + ORPHANED_CARD_BASE_PX + + staleReviewRowHeight(text, wrapWidth, STALE_REVIEW_DISCARD_WIDTH_PX), + 0, + ); + }); +} + /** * Per-segment heights for a whole book, with the prefix sums that turn a scroll offset into a * segment index and back. */ export type HeightTable = Readonly<{ - /** Height of each segment, index-aligned with the book's segment list, including the gap above it. */ + /** + * Height of each segment, index-aligned with the book's segment list, including the gap above it + * and the orphaned-translation cards beside it. + */ heights: readonly number[]; + /** Height of each segment's own box, without the gap above it or the cards beside it. */ + boxHeights: readonly number[]; /** * Running top edge of each segment, with a trailing entry for the bottom of the last one, so * `offsets` is always one longer than {@link HeightTable.heights}. @@ -233,21 +268,24 @@ export type HeightTable = Readonly<{ * @param config - Supplies the gaps charged between segments. * @param predictedHeights - Predicted height of each segment, index-aligned with the book's * segments, as {@link predictSegmentHeights} builds it. + * @param cardHeights - Predicted height of the cards beside each segment, as + * {@link predictOrphanedCardHeights} builds it. */ export function buildHeightTable( config: HeightConfig, predictedHeights: readonly number[], + cardHeights: readonly number[] = [], ): HeightTable { const heights: number[] = []; const offsets: number[] = [0]; predictedHeights.forEach((predicted, index) => { // The gap above a segment belongs to it, leaving nothing above the first. const gap = index === 0 ? 0 : (config.segmentGapPx ?? 0) + (config.extraGapPx?.(index) ?? 0); - const height = gap + predicted; + const height = gap + predicted + (cardHeights[index] ?? 0); heights.push(height); offsets.push(offsets[offsets.length - 1] + height); }); - return { heights, offsets, total: offsets[offsets.length - 1] }; + return { heights, boxHeights: predictedHeights, offsets, total: offsets[offsets.length - 1] }; } /** diff --git a/src/utils/stale-free-translations.ts b/src/utils/stale-free-translations.ts index 17f682bd9..53404976a 100644 --- a/src/utils/stale-free-translations.ts +++ b/src/utils/stale-free-translations.ts @@ -99,51 +99,97 @@ function placesInDocumentOrder(placesByVerse: ReadonlyMap): Ver .sort((a, b) => a.place.order - b.place.order); } -/** - * Picks the place a translation of a verse the book no longer holds falls back to: the last place - * of the nearest earlier verse, else the book's first place, else `undefined` for a book with - * none. - */ -function placeBefore(verse: string, inOrder: readonly VersePlace[]): Place | undefined { - const { chapterNum, verseNum } = verseOfId(verse); +/** Finds the last place of the nearest earlier verse, `undefined` where none precedes it. */ +function placeBefore( + { chapterNum, verseNum }: SerializedVerseRef, + inOrder: readonly VersePlace[], +): Place | undefined { const isEarlier = (entry: VersePlace) => entry.chapterNum < chapterNum || (entry.chapterNum === chapterNum && entry.verseNum < verseNum); - return (inOrder.findLast(isEarlier) ?? inOrder[0])?.place; + return inOrder.findLast(isEarlier)?.place; +} + +/** Reads the chapter and verse a segment id falls in, e.g. `1:5` or `1:5-6`. */ +export function verseLabelOf(segmentId: string): string { + const { verse } = positionOf(segmentId); + return verse.slice(verse.indexOf(' ') + 1).replace(/\/.*$/, ''); +} + +/** A book's stale free translations, filed by where the view shows each. */ +export type PlacedStaleFreeTranslations = Readonly<{ + /** Those each segment reviews beside its own translation, by segment id, in document order. */ + bySegment: ReadonlyMap; + /** + * Those of verses the book no longer holds, by the id of the segment ending the nearest earlier + * verse, in verse order. + */ + orphanedAfter: ReadonlyMap; + /** Those of verses the book no longer holds that ran ahead of every verse it does. */ + orphanedAhead: readonly StaleFreeTranslation[]; +}>; + +function fileUnder( + map: Map, + key: string, + translation: StaleFreeTranslation, +) { + const list = map.get(key) ?? []; + list.push(translation); + map.set(key, list); } /** - * Files each of the book's stale free translations under the segment that shows it: the segment it - * was written for, or, where that segment has since vanished, the one now covering the position it - * began at. A vanished heading's translation goes to its verse's heading of the same marker, else - * the start of its verse. A translation whose verse the book no longer holds goes to the segment - * ending the nearest earlier verse, else to the book's first segment. Each segment's list runs in - * document order of those positions. + * Files each of the book's stale free translations where the view shows it. One whose segment still + * stands goes to that segment, and one whose segment has since vanished to the segment now covering + * the position it began at. A vanished heading's translation goes to its verse's heading of the + * same marker, else the start of its verse. A translation whose verse the book no longer holds is + * orphaned, following the segment that ends the nearest earlier verse. */ export function placeStaleFreeTranslations( stale: readonly StaleFreeTranslation[], book: Book, -): ReadonlyMap { +): PlacedStaleFreeTranslations { const placesByVerse = indexPlacesByVerse(book); let inOrder: readonly VersePlace[] | undefined; - const placed = stale.flatMap((translation) => { - if (bookOfRef(translation.segmentId) !== book.bookRef) return []; + const placed: { translation: StaleFreeTranslation; covering: Place }[] = []; + const orphaned: { + translation: StaleFreeTranslation; + after: Place | undefined; + verse: SerializedVerseRef; + }[] = []; + stale.forEach((translation) => { + if (bookOfRef(translation.segmentId) !== book.bookRef) return; const { verse, offset } = positionOf(translation.segmentId); const places = placesByVerse.get(verse) ?? vanishedHeadingPlaces(verse, placesByVerse); - const covering = places - ? (places.findLast((place) => place.offset <= offset) ?? places[0]) - : placeBefore(verse, (inOrder ??= placesInDocumentOrder(placesByVerse))); - return covering ? [{ translation, covering }] : []; + if (places) { + const covering = places.findLast((place) => place.offset <= offset) ?? places[0]; + placed.push({ translation, covering }); + return; + } + inOrder ??= placesInDocumentOrder(placesByVerse); + // A book holding no text has nothing to show an orphan beside. + if (inOrder.length === 0) return; + const verseRef = verseOfId(verse); + orphaned.push({ translation, after: placeBefore(verseRef, inOrder), verse: verseRef }); }); const bySegment = new Map(); placed .toSorted((a, b) => a.covering.order - b.covering.order) - .forEach(({ translation, covering }) => { - const list = bySegment.get(covering.segmentId) ?? []; - list.push(translation); - bySegment.set(covering.segmentId, list); + .forEach(({ translation, covering }) => fileUnder(bySegment, covering.segmentId, translation)); + + const orphanedAfter = new Map(); + const orphanedAhead: StaleFreeTranslation[] = []; + orphaned + .toSorted( + (a, b) => a.verse.chapterNum - b.verse.chapterNum || a.verse.verseNum - b.verse.verseNum, + ) + .forEach(({ translation, after }) => { + if (after) fileUnder(orphanedAfter, after.segmentId, translation); + else orphanedAhead.push(translation); }); - return bySegment; + + return { bySegment, orphanedAfter, orphanedAhead }; } /** From 1c400bcb28527e99688d58bcca84f8b4a61d2e3a Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 13 Oct 2026 15:21:57 -0600 Subject: [PATCH 08/10] Don't orphan free translations across verse bridging --- .../components/AnalysisCatalogPanel.test.tsx | 14 ++-- .../components/ConcordancePanel.test.tsx | 11 ++-- .../OrphanedFreeTranslationCard.test.tsx | 9 +-- .../components/SourceTextContext.test.tsx | 64 ------------------- .../utils/stale-free-translations.test.ts | 44 +++++++++++++ .../__mocks__/TextReadingStatus.tsx | 10 +-- src/utils/analysis-query.ts | 7 +- src/utils/stale-free-translations.ts | 42 ++++++++++-- src/utils/verse-ref.ts | 2 +- 9 files changed, 105 insertions(+), 98 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 90fe3fdf8..7ad4da514 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -20,7 +20,7 @@ import type { HeadingPlacement } from '../../utils/analysis-query'; import { defaultScrRef, FIXTURE_STAMPS, makeScrollGroupHook } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; -// How reading progress and failure read is the status's own concern. +// What the reading status shows is its own concern; these cover only when the panel shows it. jest.mock('../../components/TextReadingStatus'); const NO_HEADINGS: ReadonlyMap = new Map(); @@ -2242,23 +2242,23 @@ describe('AnalysisCatalogPanel', () => { }); it('shows how far reading the text has got while filtering for them', async () => { - const text = makeText({ status: 'loading', booksRead: 12, bookCount: 66 }); - renderPanel({ analysis, text }); + renderPanel({ analysis, text: makeText({ status: 'loading' }) }); await openFilters(); await userEvent.click(notInTextFilter()); - expect(screen.getByTestId('text-reading-status')).toHaveTextContent('12 of 66'); + expect(screen.getByTestId('text-reading-status')).toBeInTheDocument(); expect(screen.queryByTestId('catalog-row')).not.toBeInTheDocument(); }); - it('says so when the text could not be read', async () => { - renderPanel({ analysis, text: makeText({ status: 'error' }) }); + it('says so when a book could not be read', async () => { + renderPanel({ analysis, text: makeText({ status: 'ready', isPartial: true }) }); await openFilters(); await userEvent.click(notInTextFilter()); - expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-status', 'error'); + expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-partial', 'true'); + expect(screen.queryByTestId('catalog-row')).not.toBeInTheDocument(); }); it('marks an analysis whose form the text lacks once it is read', () => { diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index 9f3ae5b4d..f116273a9 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -17,7 +17,7 @@ import { buildConcordanceEntries, indexBook, type ConcordanceEntry } from '../.. import { defaultScrRef, FIXTURE_STAMPS, makeScrollGroupHook, makeVerseBook } from '../test-helpers'; import { mockKeyAsValueLocalizedStrings } from './test-helpers'; -// How reading progress and failure read is the status's own concern. +// What the reading status shows is its own concern; these cover only when the panel shows it. jest.mock('../../components/TextReadingStatus'); /** @@ -138,7 +138,6 @@ function rowFor(form: string): HTMLElement { beforeEach(() => { mockKeyAsValueLocalizedStrings({ - '%interlinearizer_textReading_loading%': 'Reading {read} of {total}', '%interlinearizer_concordance_occurrenceCountInBook%': 'in {book}', }); claimedFocusRequest = undefined; @@ -146,16 +145,18 @@ beforeEach(() => { describe('ConcordancePanel', () => { it('shows how far reading the books has got while the index is built', () => { - renderPanel({ text: makeText({ status: 'loading', booksRead: 12, bookCount: 66 }) }); + renderPanel({ text: makeText({ status: 'loading' }) }); - expect(screen.getByTestId('text-reading-status')).toHaveTextContent('12 of 66'); + expect(screen.getByTestId('text-reading-status')).toBeInTheDocument(); expect(screen.queryByTestId('concordance-row')).not.toBeInTheDocument(); }); it('says so when the books could not be listed', () => { renderPanel({ text: makeText({ status: 'error' }) }); - expect(screen.getByTestId('text-reading-status')).toHaveAttribute('data-status', 'error'); + expect(screen.getByTestId('text-reading-status')).toBeInTheDocument(); + expect(screen.queryByTestId('concordance-row')).not.toBeInTheDocument(); + expect(screen.queryByTestId('concordance-empty')).not.toBeInTheDocument(); }); it('says so when the text has no words', () => { diff --git a/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx index 9251980b4..c3622b867 100644 --- a/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx +++ b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx @@ -13,11 +13,6 @@ jest.mock('../../components/AnalysisStore', () => ({ useStaleFreeTranslationDispatch: () => ({ keep: jest.fn(), discard: mockDiscard }), })); -jest.mock('../../utils/stale-free-translations', () => ({ - __esModule: true, - verseLabelOf: (segmentId: string) => `verse of ${segmentId}`, -})); - const STRINGS = { '%interlinearizer_freeTranslationInput_orphaned%': 'Gone: {verse}', '%interlinearizer_freeTranslationInput_staleDiscard%': 'Discard', @@ -36,9 +31,7 @@ describe('OrphanedFreeTranslationCard', () => { it('names the verse the translation was written for', () => { renderCard(); - expect(screen.getByTestId('orphaned-free-translation')).toHaveTextContent( - 'Gone: verse of GEN 1:5:12', - ); + expect(screen.getByText('Gone: 1:5')).toBeInTheDocument(); }); it("shows the translation's text", () => { diff --git a/src/__tests__/components/SourceTextContext.test.tsx b/src/__tests__/components/SourceTextContext.test.tsx index 9df070b6c..a5eed7055 100644 --- a/src/__tests__/components/SourceTextContext.test.tsx +++ b/src/__tests__/components/SourceTextContext.test.tsx @@ -1,74 +1,10 @@ /// import { renderHook } from '@testing-library/react'; -import type { ReactNode } from 'react'; import { - SourceTextProvider, useConcordanceEntriesContext, useSourceTextContext, } from '../../components/SourceTextContext'; -import useConcordanceEntries from '../../hooks/useConcordanceEntries'; -import useSourceTextReader, { type SourceText } from '../../hooks/useSourceTextReader'; -import type { ConcordanceEntry } from '../../utils/concordance'; -import { GEN_1_1_BOOK } from '../test-helpers'; - -// Reading books and building entries are the hooks' own concerns; the provider only hands on what -// they build. -jest.mock('../../hooks/useConcordanceEntries'); -jest.mock('../../hooks/useSourceTextReader'); - -const TEXT: SourceText = { - status: 'ready', - booksRead: 1, - bookCount: 1, - readings: new Map(), - liveVersions: new Map(), - isPartial: false, - textForms: undefined, - refresh: () => {}, - request: () => {}, -}; - -const ENTRIES: readonly ConcordanceEntry[] = []; - -/** Wraps a hook in a provider with GEN 1:1 live and the concordance shown. */ -function wrapper({ children }: Readonly<{ children: ReactNode }>) { - return ( - - {children} - - ); -} - -beforeEach(() => { - jest.mocked(useSourceTextReader).mockReturnValue(TEXT); - jest.mocked(useConcordanceEntries).mockReturnValue(ENTRIES); -}); - -describe('SourceTextProvider', () => { - it('reads the text from its props and hands it to what it wraps', () => { - const { result } = renderHook(() => useSourceTextContext(), { wrapper }); - - expect(result.current).toBe(TEXT); - expect(useSourceTextReader).toHaveBeenCalledWith({ - enabled: true, - liveBook: GEN_1_1_BOOK, - projectId: 'src', - writingSystem: 'en', - }); - }); - - it('builds the entries from the text it read and hands them to what it wraps', () => { - const { result } = renderHook(() => useConcordanceEntriesContext(), { wrapper }); - - expect(result.current).toBe(ENTRIES); - expect(useConcordanceEntries).toHaveBeenCalledWith({ - text: TEXT, - shown: true, - writingSystem: 'en', - }); - }); -}); describe('useSourceTextContext', () => { it('throws outside a provider', () => { diff --git a/src/__tests__/utils/stale-free-translations.test.ts b/src/__tests__/utils/stale-free-translations.test.ts index cb4c6445e..b05393bfd 100644 --- a/src/__tests__/utils/stale-free-translations.test.ts +++ b/src/__tests__/utils/stale-free-translations.test.ts @@ -127,6 +127,50 @@ describe('placeStaleFreeTranslations', () => { expect(placed([stale('GEN 1:1/s1')], book)).toEqual({ 'GEN 1:1': ['sa GEN 1:1/s1'] }); }); + it('files a translation of a verse since bridged under the bridge', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:2-3', text: 'and the earth was void and God said' }, + ]); + + expect(placed([stale('GEN 1:3')], book)).toEqual({ 'GEN 1:2-3': ['sa GEN 1:3'] }); + }); + + it('files a translation begun mid-verse in a verse since bridged under the start of the bridge', () => { + const book = resegmentBook( + makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 1:2-3', text: 'and the earth was void and God said' }, + ]), + { removedVerseStarts: [], addedStarts: [{ tokenRef: 'GEN 1:2-3:4', surfaceText: 'the' }] }, + ); + + expect(placed([stale('GEN 1:3:4')], book)).toEqual({ 'GEN 1:2-3': ['sa GEN 1:3:4'] }); + }); + + it('files a translation of a bridge since split under its first verse', () => { + expect(placed([stale('GEN 1:2-3')])).toEqual({ 'GEN 1:2': ['sa GEN 1:2-3'] }); + }); + + it('files a translation of a verse since bridged under the bridge rather than a heading in it', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { heading: 's1', verseId: 'GEN 1:2-3', text: 'Title', charIndex: 0 }, + { sid: 'GEN 1:2-3', text: 'and the earth was void and God said' }, + ]); + + expect(placed([stale('GEN 1:3')], book)).toEqual({ 'GEN 1:2-3': ['sa GEN 1:3'] }); + }); + + it("orphans a translation of a deleted verse that another chapter's bridge numbers", () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', text: 'In the beginning' }, + { sid: 'GEN 2:2-3', text: 'And on the seventh day' }, + ]); + + expect(orphanedAfter([stale('GEN 1:3')], book)).toEqual({ 'GEN 1:1': ['sa GEN 1:3'] }); + }); + it('orphans a translation of a deleted verse after the segment ending the verse before it', () => { const book = resegmentBook( makeVerseBook([ diff --git a/src/components/__mocks__/TextReadingStatus.tsx b/src/components/__mocks__/TextReadingStatus.tsx index 49f393b93..eee993f9c 100644 --- a/src/components/__mocks__/TextReadingStatus.tsx +++ b/src/components/__mocks__/TextReadingStatus.tsx @@ -5,10 +5,12 @@ export const { TEXT_READING_STRING_KEYS } = jest.requireActual('../TextReadingSt /** Stands in for the reading status, exposing the reading state it was handed. */ export default function TextReadingStatus({ text, -}: Readonly<{ text: Pick }>) { +}: Readonly<{ text: Pick }>) { return ( -

    - {`${text.booksRead} of ${text.bookCount}`} -

    +

    ); } diff --git a/src/utils/analysis-query.ts b/src/utils/analysis-query.ts index 52501e40d..d0fd80feb 100644 --- a/src/utils/analysis-query.ts +++ b/src/utils/analysis-query.ts @@ -104,7 +104,10 @@ export interface CatalogFilters { zeroUsages?: boolean; /** Keeps only rows with a stale location, in one of the selected books where `books` is active. */ stale?: boolean; - /** Keeps only rows whose form the query's `textForms` lacks, and none while the text is unread. */ + /** + * Keeps only rows whose form isn't in {@link CatalogQuery.textForms}, and none while that is + * unset. + */ notInText?: boolean; /** Keeps only rows with no gloss in the scope's analysis language. */ missingGloss?: boolean; @@ -135,7 +138,7 @@ export interface CatalogQuery { surfaceCollator: Collator; /** Collates glosses, so ordering follows the analysis language rather than code points. */ glossCollator: Collator; - /** Every normalized word form the source text holds, `undefined` while it is unread. */ + /** Every normalized word form the source text holds, `undefined` until every book has been read. */ textForms?: ReadonlySet; } diff --git a/src/utils/stale-free-translations.ts b/src/utils/stale-free-translations.ts index 53404976a..d09b4aab7 100644 --- a/src/utils/stale-free-translations.ts +++ b/src/utils/stale-free-translations.ts @@ -2,7 +2,7 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Book, Segment } from 'interlinearizer'; import { bookOfRef } from './analysis-book'; import { verseOfTokenRef } from './reanchor-analysis'; -import { segmentContainsVerse, verseOfId } from './verse-ref'; +import { segmentContainsVerse, verseLabelCovers, verseOfId } from './verse-ref'; /** A free translation whose segment's text has changed since it was written. */ export type StaleFreeTranslation = Readonly<{ @@ -86,8 +86,31 @@ function vanishedHeadingPlaces( ); } -/** A place with the chapter and first verse number of the verse it falls in. */ -type VersePlace = Readonly<{ place: Place; chapterNum: number; verseNum: number }>; +/** + * Finds the start of the verse now holding a vanished verse's first verse, as `GEN 1:3-4` holds + * `GEN 1:4`, else `undefined` where no verse of the book holds it. + */ +function coveringVerseStart( + verse: string, + placesByVerse: ReadonlyMap, +): Place | undefined { + const { chapterNum, verseNum } = verseOfId(verse); + const covering = [...placesByVerse].find( + ([key]) => + !key.includes('/') && + verseOfId(key).chapterNum === chapterNum && + verseLabelCovers(key.slice(key.indexOf(':') + 1), verseNum), + ); + return covering?.[1][0]; +} + +/** A place, with the chapter and verse it falls in. */ +type VersePlace = Readonly<{ + place: Place; + chapterNum: number; + /** Its verse number, a bridged verse's first. */ + verseNum: number; +}>; /** Lists every place in the book in document order, each with the verse it falls in. */ function placesInDocumentOrder(placesByVerse: ReadonlyMap): VersePlace[] { @@ -142,8 +165,10 @@ function fileUnder( * Files each of the book's stale free translations where the view shows it. One whose segment still * stands goes to that segment, and one whose segment has since vanished to the segment now covering * the position it began at. A vanished heading's translation goes to its verse's heading of the - * same marker, else the start of its verse. A translation whose verse the book no longer holds is - * orphaned, following the segment that ends the nearest earlier verse. + * same marker, else the start of its verse. One whose verse the book holds only under another verse + * marker, joined into a bridge or split out of one, goes to the start of that marker's verse. A + * translation whose verse the book no longer holds is orphaned, following the segment that ends the + * nearest earlier verse. */ export function placeStaleFreeTranslations( stale: readonly StaleFreeTranslation[], @@ -161,8 +186,11 @@ export function placeStaleFreeTranslations( if (bookOfRef(translation.segmentId) !== book.bookRef) return; const { verse, offset } = positionOf(translation.segmentId); const places = placesByVerse.get(verse) ?? vanishedHeadingPlaces(verse, placesByVerse); - if (places) { - const covering = places.findLast((place) => place.offset <= offset) ?? places[0]; + // An offset names no position in a verse its own has since been bridged into or split from. + const covering = places + ? (places.findLast((place) => place.offset <= offset) ?? places[0]) + : coveringVerseStart(verse, placesByVerse); + if (covering) { placed.push({ translation, covering }); return; } diff --git a/src/utils/verse-ref.ts b/src/utils/verse-ref.ts index 6266c3f17..a6baf2ffd 100644 --- a/src/utils/verse-ref.ts +++ b/src/utils/verse-ref.ts @@ -40,7 +40,7 @@ export function firstVerseNumber(verseStartNumber: string): number | undefined { * Whether a verbatim USJ verse label names the given verse. A range matches any verse between its * endpoints inclusive; a label that parses to no digits matches nothing. */ -function verseLabelCovers(verseStartNumber: string, verseNum: number): boolean { +export function verseLabelCovers(verseStartNumber: string, verseNum: number): boolean { const range = parseVerseLabel(verseStartNumber); if (!range) return false; return verseNum >= range.first && verseNum <= range.last; From 994527d4d75e5f6ddad55480371d5ff9b2aed6f0 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 14 Oct 2026 13:59:57 -0600 Subject: [PATCH 09/10] Truncate orphaned translation cards to a fixed height --- .../components/Interlinearizer.test.tsx | 15 +++----- src/__tests__/utils/segment-heights.test.ts | 38 +++++-------------- .../OrphanedFreeTranslationCard.tsx | 7 ++-- src/components/SegmentListView.tsx | 16 ++++---- src/hooks/useSegmentHeights.ts | 4 +- src/utils/segment-heights.ts | 20 ++++------ 6 files changed, 36 insertions(+), 64 deletions(-) diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 5d4a5b871..fed871e45 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -1619,20 +1619,17 @@ describe('Interlinearizer', () => { { analysisId: 'sa-2', segmentId: 'GEN 1:190:9', text: 'B' }, ]); - // Each card's gap, chrome, and row. - expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(2 * (8 + 34 + 32)); + // Each card's gap and height. + expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(2 * (8 + 66)); }); - it('reserves the placeholder an orphaned translation with no text in the language shows', () => { - mockKeyAsValueLocalizedStrings({ - '%interlinearizer_freeTranslationInput_staleNoText%': 'a'.repeat(200), - }); + it('reserves a long orphaned translation no more than a short one', () => { const without = trailingSpacerPx(STALE_BELOW_WINDOW); mockOrphanedAfter.set('GEN 1:190', [ - { analysisId: 'sa-1', segmentId: 'GEN 1:190:9', text: '' }, + { analysisId: 'sa-1', segmentId: 'GEN 1:190:9', text: 'a'.repeat(200) }, ]); - expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(8 + 34 + 32 + 2 * 20 + 4); + expect(trailingSpacerPx(STALE_BELOW_WINDOW) - without).toBe(8 + 66); }); it('reserves the cards orphaned ahead of the book above the window', () => { @@ -1647,7 +1644,7 @@ describe('Interlinearizer', () => { const without = leadingSpacerPx(); mockOrphanedAhead.push({ analysisId: 'sa-1', segmentId: 'GEN 1:0', text: 'A' }); - expect(leadingSpacerPx() - without).toBe(8 + 34 + 32); + expect(leadingSpacerPx() - without).toBe(8 + 66); }); it('reserves no orphaned cards for a read-only analysis', () => { diff --git a/src/__tests__/utils/segment-heights.test.ts b/src/__tests__/utils/segment-heights.test.ts index 9911a519f..11d978689 100644 --- a/src/__tests__/utils/segment-heights.test.ts +++ b/src/__tests__/utils/segment-heights.test.ts @@ -333,49 +333,31 @@ describe('predictOrphanedCardHeights', () => { const withCards = { ...CONFIG, showFreeTranslation: true, segmentGapPx: 8 }; it('charges nothing beside a segment showing no cards', () => { - expect(predictOrphanedCardHeights(2, withCards, 300)).toEqual([0, 0]); + expect(predictOrphanedCardHeights(2, withCards)).toEqual([0, 0]); }); - it('charges each card its chrome, its row, and the gap beside it', () => { - const config = { ...withCards, orphanedTranslationTexts: () => ['A', 'B'] }; + it('charges each card its height and the gap beside it', () => { + const config = { ...withCards, orphanedCardCount: () => 2 }; - expect(predictOrphanedCardHeights(1, config, 300)).toEqual([2 * (8 + 34 + 32)]); + expect(predictOrphanedCardHeights(1, config)).toEqual([2 * (8 + 66)]); }); it('charges no gap beside a card where the config sets none', () => { - const config = { - ...CONFIG, - showFreeTranslation: true, - orphanedTranslationTexts: () => ['A'], - }; + const config = { ...CONFIG, showFreeTranslation: true, orphanedCardCount: () => 1 }; - expect(predictOrphanedCardHeights(1, config, 300)).toEqual([34 + 32]); + expect(predictOrphanedCardHeights(1, config)).toEqual([66]); }); it('charges only the segments showing cards', () => { - const config = { - ...withCards, - orphanedTranslationTexts: (index: number) => (index === 1 ? ['A'] : []), - }; + const config = { ...withCards, orphanedCardCount: (index: number) => (index === 1 ? 1 : 0) }; - expect(predictOrphanedCardHeights(2, config, 300)).toEqual([0, 8 + 34 + 32]); - }); - - it('charges the wrapped lines of a card too long to share a line with its button', () => { - const config = { ...withCards, orphanedTranslationTexts: () => ['a'.repeat(200)] }; - - // 200 characters at 6.2px wrap onto five 300px lines, the button on a line below them. - expect(predictOrphanedCardHeights(1, config, 300)).toEqual([8 + 34 + 32 + 5 * 20 + 4]); + expect(predictOrphanedCardHeights(2, config)).toEqual([0, 8 + 66]); }); it('charges nothing while free translations are hidden', () => { - const config = { - ...withCards, - showFreeTranslation: false, - orphanedTranslationTexts: () => ['A'], - }; + const config = { ...withCards, showFreeTranslation: false, orphanedCardCount: () => 1 }; - expect(predictOrphanedCardHeights(1, config, 300)).toEqual([0]); + expect(predictOrphanedCardHeights(1, config)).toEqual([0]); }); }); diff --git a/src/components/OrphanedFreeTranslationCard.tsx b/src/components/OrphanedFreeTranslationCard.tsx index d0bb70c6c..147e4f6b6 100644 --- a/src/components/OrphanedFreeTranslationCard.tsx +++ b/src/components/OrphanedFreeTranslationCard.tsx @@ -31,14 +31,15 @@ export default function OrphanedFreeTranslationCard({ data-analysis-id={translation.analysisId} data-testid="orphaned-free-translation" > - + {/* Each line truncates: the list predicts every card at the same height. */} + {formatReplacementString( localizedStrings['%interlinearizer_freeTranslationInput_orphaned%'], { verse: verseLabelOf(translation.segmentId) }, )} -

    - +
    + {translation.text || localizedStrings['%interlinearizer_freeTranslationInput_staleNoText%']} diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 8e46b7e4b..141a6bd01 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -380,14 +380,12 @@ export default function SegmentListView({ [staleFreeTranslationsBySegment, segmentsWithApprovedTranslation, book.segments, staleNoText], ); - /** The text of each orphaned-translation card rendered beside a segment, ahead of it or after. */ - const orphanedTranslationTexts = useCallback( + /** How many orphaned-translation cards render beside a segment, ahead of it or after. */ + const orphanedCardCount = useCallback( (index: number) => - [ - ...(index === 0 ? orphanedAhead : []), - ...(orphanedAfter.get(book.segments[index].id) ?? []), - ].map((translation) => translation.text || staleNoText), - [orphanedAhead, orphanedAfter, book.segments, staleNoText], + (index === 0 ? orphanedAhead.length : 0) + + (orphanedAfter.get(book.segments[index].id)?.length ?? 0), + [orphanedAhead, orphanedAfter, book.segments], ); /** Whether a segment's review rows offer Keep, which they do only while it has no approval. */ @@ -423,7 +421,7 @@ export default function SegmentListView({ freeTranslationText: readOnly ? freeTranslationText : undefined, staleReviewTexts: readOnly ? undefined : staleReviewTexts, staleReviewOffersKeep, - orphanedTranslationTexts: readOnly ? undefined : orphanedTranslationTexts, + orphanedCardCount: readOnly ? undefined : orphanedCardCount, segmentGapPx: SEGMENT_ROW_GAP_PX, extraGapPx, }), @@ -435,7 +433,7 @@ export default function SegmentListView({ freeTranslationText, staleReviewTexts, staleReviewOffersKeep, - orphanedTranslationTexts, + orphanedCardCount, extraGapPx, ], ); diff --git a/src/hooks/useSegmentHeights.ts b/src/hooks/useSegmentHeights.ts index a540ba93a..4d8a63da6 100644 --- a/src/hooks/useSegmentHeights.ts +++ b/src/hooks/useSegmentHeights.ts @@ -65,8 +65,8 @@ export default function useSegmentHeights({ ); const cardHeights = useMemo( - () => predictOrphanedCardHeights(book.segments.length, config, wrapWidth), - [book.segments.length, config, wrapWidth], + () => predictOrphanedCardHeights(book.segments.length, config), + [book.segments.length, config], ); return useMemo( diff --git a/src/utils/segment-heights.ts b/src/utils/segment-heights.ts index 8a6c9efe5..1a1667a0d 100644 --- a/src/utils/segment-heights.ts +++ b/src/utils/segment-heights.ts @@ -51,8 +51,8 @@ const STALE_REVIEW_GAP_PX = 4; /** Height of one line of stale translation text listed for review. */ const STALE_REVIEW_TEXT_LINE_PX = 20; -/** Orphaned-translation card chrome above its row: its border, padding, and heading line. */ -const ORPHANED_CARD_BASE_PX = 34; +/** Height of an orphaned-translation card, which holds its heading and its text to a line apiece. */ +const ORPHANED_CARD_PX = 66; /** Width assumed for one character of plain baseline text, in pixels. */ const BASELINE_CHAR_PX = 8.4; @@ -86,10 +86,10 @@ export type HeightConfig = Readonly<{ */ staleReviewOffersKeep?: (index: number) => boolean; /** - * The text of each card the list renders beside the segment at `index` for a stale translation of - * a verse the book no longer holds. Defaults to none. + * How many cards the list renders beside the segment at `index` for stale translations of verses + * the book no longer holds. Defaults to none. */ - orphanedTranslationTexts?: (index: number) => readonly string[]; + orphanedCardCount?: (index: number) => number; /** Which renderer the segment uses; `baseline-text` has no chips and so no rows. */ displayMode: 'token-chip' | 'baseline-text'; /** @@ -226,17 +226,11 @@ export function predictSegmentHeights( export function predictOrphanedCardHeights( segmentCount: number, config: HeightConfig, - wrapWidth: number, ): readonly number[] { return Array.from({ length: segmentCount }, (_unused, index) => { if (!config.showFreeTranslation) return 0; - return (config.orphanedTranslationTexts?.(index) ?? []).reduce( - (height, text) => - height + - (config.segmentGapPx ?? 0) + - ORPHANED_CARD_BASE_PX + - staleReviewRowHeight(text, wrapWidth, STALE_REVIEW_DISCARD_WIDTH_PX), - 0, + return ( + (config.orphanedCardCount?.(index) ?? 0) * ((config.segmentGapPx ?? 0) + ORPHANED_CARD_PX) ); }); } From ece6bea627f46291b17d5e816314d038fea4f4bd Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 14 Oct 2026 16:21:19 -0600 Subject: [PATCH 10/10] Show an orphaned translation's full text in a tooltip --- .../OrphanedFreeTranslationCard.test.tsx | 13 ++++++++++++- src/components/OrphanedFreeTranslationCard.tsx | 15 ++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx index c3622b867..2fbf6bfc3 100644 --- a/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx +++ b/src/__tests__/components/OrphanedFreeTranslationCard.test.tsx @@ -5,6 +5,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import OrphanedFreeTranslationCard from '../../components/OrphanedFreeTranslationCard'; import type { StaleFreeTranslation } from '../../utils/stale-free-translations'; +import { withTooltipProvider } from './test-helpers'; const mockDiscard = jest.fn(); @@ -23,7 +24,9 @@ const STRINGS = { function renderCard(text = 'Au début') { const translation: StaleFreeTranslation = { analysisId: 'sa-1', segmentId: 'GEN 1:5:12', text }; return render( - , + withTooltipProvider( + , + ), ); } @@ -40,6 +43,14 @@ describe('OrphanedFreeTranslationCard', () => { expect(screen.getByTestId('orphaned-free-translation')).toHaveTextContent('Au début'); }); + it("offers the translation's full text in a tooltip", () => { + const longText = + 'In the beginning God created the heavens and the earth, and the earth was void'; + renderCard(longText); + + expect(screen.getByText(longText)).toHaveAttribute('title', longText); + }); + it('shows the placeholder for a translation with no text in the language', () => { renderCard(''); diff --git a/src/components/OrphanedFreeTranslationCard.tsx b/src/components/OrphanedFreeTranslationCard.tsx index 147e4f6b6..15cba378a 100644 --- a/src/components/OrphanedFreeTranslationCard.tsx +++ b/src/components/OrphanedFreeTranslationCard.tsx @@ -1,4 +1,4 @@ -import { Button } from 'platform-bible-react'; +import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'platform-bible-react'; import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; import { useStaleFreeTranslationDispatch } from './AnalysisStore'; import { verseLabelOf, type StaleFreeTranslation } from '../utils/stale-free-translations'; @@ -25,6 +25,8 @@ export default function OrphanedFreeTranslationCard({ localizedStrings, }: OrphanedFreeTranslationCardProps) { const { discard } = useStaleFreeTranslationDispatch(); + const text = + translation.text || localizedStrings['%interlinearizer_freeTranslationInput_staleNoText%']; return (
    - - {translation.text || - localizedStrings['%interlinearizer_freeTranslationInput_staleNoText%']} - + {/* The tooltip is the only place a truncated translation can be read in full. */} + + + {text} + + {text} +