PT-3976: Add ResourcePickerDialog to platform-bible-react and register as paranext-core dialog

[captaincrazybro]

Code Review Summary — Round 2

Branch: pt-3976-shared-resource-picker-ui

Base: origin/main

Date: 2026-05-13

Review model: Claude Sonnet 4.6

Files changed: 19

Overview

This round adds a useProgressiveList hook backed by IntersectionObserver to the "Available to Download" section of ResourcePickerDialog, rendering items 50 at a time as the user scrolls rather than all at once. A LargeResourceList Storybook story demonstrates the optimization at 2500 entries. The PR also enhances the language filter with a multi-select MultiSelectComboBox that shows custom summary text (%resourcePicker_language_filter_multipleSelected%) when multiple languages are chosen. Several issues were caught and fixed during review, including a missing Spanish translation, stale dist/ artifacts, a missing eslint-disable explanation, missing useMemo deps, redundant type casts, and a missing aria-hidden on the scroll sentinel.

API Changes

• useProgressiveList<T>(items, pageSize?) — new exported hook from lib/platform-bible-react, returns { visibleItems, sentinelRef, hasMore }
• LARGE_SAMPLE_RESOURCES — new exported constant (2500 generated DblResourceData entries) from resource-picker-dialog.data.ts
• New localization keys in en.json and es.json: %resourcePicker_language_filter_any%, %resourcePicker_language_filter_multipleSelected%, %resourcePicker_showing_count%

Findings

Important — Fixed During Review

• assets/localization/es.json: %resourcePicker_language_filter_multipleSelected% was added to en.json but missing from es.json. (fixed: added "{selectCount} idiomas")
• resource-picker-dialog.component.tsx: customLanguageSelectText useMemo dependency array omitted anyLanguageText and localizedStrings. (fixed: added both to deps)
• resource-picker-dialog.utils.test.ts: Three as IntersectionObserverEntry casts were redundant — ioCallback is already typed as Partial<IntersectionObserverEntry>[]. (fixed: removed all three casts)
• resource-picker-dialog.utils.ts: eslint-disable-next-line no-null/no-null had no explanatory comment. (fixed by author: added explanation)
• lib/platform-bible-react/dist/: Build artifacts were stale, missing new hook export and localization key. (fixed by author: ran npm run build)
• resource-picker-dialog.utils.ts: useProgressiveList had no JSDoc — sentinel placement constraint and memoization requirement were undocumented. (fixed: added full JSDoc)
• resource-picker-dialog.component.tsx: Scroll sentinel <div ref={sentinelRef} /> was not hidden from assistive technology. (fixed: added aria-hidden)
• resource-picker-dialog.stories.tsx: LargeResourceList story had no explicit name; purpose was not clear from auto-generated label. (fixed: added name: 'Large Resource List (2500 entries)')

Important — Deferred by Author

• "Use" button label on uninstalled resources (author: intentional — consistent label across both sections)

Minor — No Action Taken

• toDownload useMemo may be missing selectedLanguages dep
• No keyboard shortcut to clear all selected languages
• LARGE_SAMPLE_RESOURCES generated at module load time (minor test startup cost)
• useProgressiveList not re-exported from library barrel (author: internal API, not needed)

Positive Observations

• Clean separation: useProgressiveList in its own utils.ts with 5 focused unit tests
• 22 tests across 3 test files (hook unit, component integration, Storybook interaction)
• sentinelRef null guard (if (el) observer.observe(el)) handles jsdom environment in tests without special-casing
• Generated data for Storybook uses cycling languages/types — no 2500-line literal in source
• Reset-on-filter relies on memoized array reference rather than deep equality — clean and predictable

Suggested Review Focus

• toDownload useMemo deps: Should selectedLanguages be in the dependency array?
• "Use" label for uninstalled resources: Acceptable UX, or should uninstalled resources say "Download"?

---

Code Review Summary — Round 1

Branch: pt-3976-shared-resource-picker-ui

Base: origin/main

Date: 2026-05-12

Review model: Claude Sonnet 4.6

Files changed: 18

Overview

This branch adds a ResourcePickerDialog component to platform-bible-react and registers it as a new dialog in paranext-core. The component is a presentational dialog that displays DBL resources in three sections — Already selected, Installed, and Available to download — and supports text search and language filtering. It is intended to be consumed by upcoming tickets that let users choose resources to support Bible translation.

The implementation follows established platform patterns: a pure presentational component in platform-bible-react receives all data via props with no PAPI coupling, while a thin wrapper in paranext-core wires up useLocalizedStrings and registers the dialog via the existing DIALOGS map. Several issues were caught and fixed during review, including a stale papi.d.ts, alphabetical ordering violations in localization files, duplicate JSX sections, a broken Storybook story, and title-case violations.

API Changes

• lib/platform-bible-react — Added ResourcePickerDialog component (default export)
• lib/platform-bible-react — Added ResourcePickerDialogProps type export
• lib/platform-bible-react — Added ResourcePickerDialogLocalizedStrings type export
• lib/platform-bible-react — Added RESOURCE_PICKER_DIALOG_STRING_KEYS constant export (9 localization keys)
• papi.d.ts / dialog-definition.model module — Added RESOURCE_PICKER_DIALOG_TYPE constant ('platform.resourcePicker')
• papi.d.ts / dialog-definition.model module — Added ResourcePickerDialogOptions type (allResources?: DblResourceData[], resourceType?: ResourceType, selectedResourceIds?: string[])
• papi.d.ts / dialog-definition.model module — Added [RESOURCE_PICKER_DIALOG_TYPE]: DialogDataTypes<ResourcePickerDialogOptions, DblResourceData> entry to DialogTypes interface

Findings

Critical — Must address before merge

• lib/papi-dts/papi.d.ts: allResources was typed as required (DblResourceData[]) but the source dialog-definition.model.ts declares it optional (DblResourceData[]?). The file had been hand-edited rather than regenerated, causing the published type contract to diverge from the implementation. (fixed during review: regenerated papi.d.ts via npm run build:types; allResources is now correctly optional)

Important — Should address before merge

• DblResourceData and ResourceType are not re-exported from @papi/core. Extension developers using showDialog('platform.resourcePicker', ...) or consuming DialogTypes['platform.resourcePicker'] will need these types. They are accessible via platform-bible-utils, but discoverability from @papi/core would follow the existing pattern used for types like LocalizeKey. Consider adding re-exports from the platform-bible-utils group in @papi/core.
• assets/localization/en.json: The nine %resourcePicker_*% keys were inserted after %resources_deprecated_any%, but alphabetically resourceP sorts before resources. (fixed during review)
• resource-picker-dialog.component.tsx: The installed and toDownload JSX sections were structurally identical (~60 lines of duplicated JSX). (fixed during review: extracted ResourceSection component)
• resource-picker-dialog.component.test.tsx: The language filter has no unit test coverage. (author dismissed)
• resource-picker-dialog.stories.tsx: The NoResults story did not render the no-results state. (fixed during review)
• Section labels used title case — violates sentence-case requirement. (fixed during review)
• hover:tw-bg-transparent on TableCell suppressed row hover highlight. (fixed during review)

Minor

• tw-z-[200] raw z-index value (dismissed)
• No DialogDescription (dismissed)
• {fullName} ({displayName}) not localizable (acknowledged, left for reviewer)
• … vs ... in search placeholder (fixed during review)
• No cancel button (intentional)
• "Use" label for uninstalled resources (deferred)

Suggested Review Focus

• DblResourceData/ResourceType discoverability from @papi/core
• Full Name (Abbrev) format localizability
• "Use" button label for uninstalled resources
• No cancel button — confirm product intent

---

This change is