Skip to content

PT-4731: Show Biblica license notice on restricted texts - #2864

Open
katherinejensen00 wants to merge 6 commits into
mainfrom
pt-4731-biblica-texts-special-message
Open

katherinejensen00 wants to merge 6 commits into
mainfrom
pt-4731-biblica-texts-special-message

Conversation

@katherinejensen00

@katherinejensen00 katherinejensen00 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Branch: pt-4731-biblica-texts-special-message

Base: origin/main

Date: 2026-09-24

Review model: Claude Opus 5.5

Files changed: 98. About 1,800 lines are production code; the rest is tests, stories, translations, test fixtures and generated files.

Summary

What users see. Biblica licenses some texts (NIV, NVI and others) as "reference text" resources only: they may not be translated or used as the basis for a new translation. When one of these resources is open, Simple and Power mode now show a dismissible banner with Biblica's wording.

  • The banner is clamped to two lines, with Show more.
  • "More info" opens a "Copyright for " dialog with Biblica's terms and a biblica.com/permissions link.
  • A narrow Text Collection cell shows a small indicator instead.
  • The same banner also shows Paratext 9's existing "Notification:" copyrights.

Second feature (the ticket's nice-to-have). These texts are greyed out, with a reason, in the pickers. They can't be chosen as a model text (resource picker, Team Layout) or as the base of a new book (Manage Books "Create based on"). An existing selection is kept, and its banner shows.

How it works. One C# check, BiblicaLicensing.IsRestricted, decides what is restricted. It applies to resources only: the DBL id must be on Matt T's list of 56 texts, or the copyright must say "Biblica, Inc." without "Biblica Open". That check reaches the UI by two paths:

  • Banner: a computed, read-only project setting, platformScripture.copyrightNotice.
  • Pickers: listModelTextRestrictions, which platform-get-resources stamps onto rows as isRestrictedAsModelText, and ProjectSummary.IsRestrictedAsBase for Manage Books.

Suggested reading order

# Area Start here What it does
1 The rule (C#) c-sharp/Projects/DigitalBibleLibrary/BiblicaLicensing.cs IsRestricted: the id list, the copyright rule and the Open-text exemptions. Tests: BiblicaLicensingTests.cs, backed by a survey of 1,819 DBL texts in TestData/.
2 The setting (C#) c-sharp/Projects/CopyrightNotice.cs, ParatextProjectDataProvider.cs Builds the none / notification / restrictedLicense value, which is read-only. The TS type is CopyrightNotice in platform-scripture.d.ts.
3 Banner UI extensions/src/platform-scripture-editor/src/copyright-notice/ project-copyright-notice.component.tsx reads the setting and handles dismissal. It renders copyright-notice-banner, copyright-details-dialog and, in narrow cells, copyright-notice-indicator.
4 Banner hosts platform-scripture-editor.web-view.tsx, model-text-panel.*, resource-text-panel.*, scripture-text-grid/resource-cell* Each places the banner. The grid also stops clicks and keys in the dialog from reaching the cell (chapter-context-escape.utils.ts).
5 Picker restriction ModelTextRestrictions.cs → platform-get-resources/src/model-text-restrictions.utils.ts → src/renderer/components/dialogs/restricted-model-text.utils.ts → resource-picker.dialog.tsx, team-layout.* Fetches the restricted ids, stamps rows, and disables them through ResourcePickerDialog's new getDisabledReason.
6 Manage Books c-sharp/ManageBooks/ProjectSummary.cs → manage-books-dialog.component.tsx → ProjectSelector IsRestrictedAsBase becomes a disabledReason. ProjectSelector disabled rows can now show their tooltip.

Safe to skim:

  • Generated: lib/*/dist/, lib/papi-dts/papi.d.ts.
  • Translations: the 33 locales in localizedStrings.json.
  • Test fixtures: the TestData/*.json files.
  • Housekeeping: the ADR entry adr-biblica-license-notice-computed-setting.

Decisions worth a second look

  • Paratext 9 parity. A "Notification:" copyright shows its banner on any project. The Biblica notice shows on resources only, since an editable project is the user's own translation. A dismissed banner comes back when the pane switches to another text and back.
  • Enforcement is UI only. Only the pickers enforce the restriction. An existing model or base selection isn't cleared.
  • No e2e test. A DBL resource can't exist in an isolated e2e profile. Unit and component tests cover the behaviour.
  • Translations are AI-generated and not native-reviewed. All 33 locales were translated this way. "Outside of Paratext" is Biblica's own wording, left verbatim.
  • Known gap, a follow-up. In Manage Books, cmdk skips disabled rows with the keyboard, so the reason there shows on hover only.

How to try it

  • Storybook: Bundled Extensions/platform-scripture-editor/CopyrightNoticeBanner has 7 stories, including the dialog, a right-to-left name and a "Notification:" text.
  • In the app: install a Biblica resource such as NIV11, then:
    1. Open it in the editor or as a model text, and check that the banner appears.
    2. Open Team Layout's model-text picker or Manage Books "Create based on", and check that it is greyed out with a reason.
Full review notes: API changes, findings, interview and quality checks

API Changes

  • lib/platform-bible-utils (resources.model.ts): DblResourceData gains an optional isRestrictedAsModelText?: boolean field.
  • lib/platform-bible-react (experimental ResourcePickerDialog): ResourcePickerDialogProps gains an optional getDisabledReason?: (resource: DblResourceData) => string | undefined. When it returns a reason for a row in the "Installed" or "Available to Download" section, that row is dimmed, shows a lock icon and a tooltip, and cannot be picked.
  • lib/platform-bible-react (experimental ProjectSelector): no signature change. Behavior changes:
    • Disabled rows now receive pointer events (tw:data-[disabled=true]:pointer-events-auto), so their disabledReason tooltip can open.
    • disabledReason is exposed to screen readers through aria-describedby.
    • The JSDoc on ProjectSelectorProject.disabledReason was updated to match.
    • Updated during review: a disabled row no longer shows the bound-but-closed "Open" button, and the isBoundButClosed JSDoc says so.
  • papi.d.ts (renderer/components/dialogs/dialog-definition.model, reached through the dialog types): ResourcePickerDialogOptions gains an optional disableRestrictedModelTexts?: boolean. It matches the source in src/renderer/components/dialogs/dialog-definition.model.ts and looks regenerated, not hand-edited.
  • extensions/src/platform-scripture/src/types/platform-scripture.d.ts:
    • New exported type CopyrightNotice, a union over kind: none, notification and restrictedLicense.
    • CopyrightNotice is added to the papi-shared-types import.
    • New project setting 'platformScripture.copyrightNotice': CopyrightNotice in ProjectSettingTypes. It is computed and read-only; C# throws InvalidOperationException on a write.
  • extensions/src/platform-get-resources/src/types/platform-get-resources.d.ts:
    • New exported type ModelTextRestrictions ({ dblIds: string[]; projectIds: string[] }).
    • New method listModelTextRestrictions(): Promise<ModelTextRestrictions> on IDblResourcesProvider.
    • getCachedResources and getLocalNonDblResources: docs now say each row carries isRestrictedAsModelText, and that a read can wait up to 2 seconds for the first restrictions fetch.
  • New localized core key %restrictedModelOrBaseText_disabledReason% (en, es).

Findings

Critical — Must address before merge

None.

Important — Should address before merge

  • The CopyrightNotice and platformScripture.copyrightNotice TSDoc said a non-resource project always gets none, but a "Notification:" copyright shows on any project, as in Paratext 9. (fixed during review: both docs now say restrictedLicense applies only to resources, notification to any project whose copyright starts with "Notification:", and otherwise none.)
  • "More info…" had an ellipsis, against the Ellipses guideline, because the dialog it opens only shows information. It also clashed with the existing "More info" labels in the same panels. (fixed during review: removed the ellipsis in all 33 locales, and updated the comments, tests and stories to match.)
  • The picker tooltip, "This text is disabled from being selected because of licensing terms which prohibit its use as a model or base for a new translation.", is one 25-word sentence, longer than the Tooltips guideline allows. (Author kept it: this is Ian's wording from the ticket.)
  • The ProjectSelector disabled-reason line used tw:text-muted-foreground on the tooltip's inverted bg-foreground: about 4.0:1 contrast in light theme and 2.5:1 in dark, below AA. This branch is the first to show text in that line. (fixed during review: dropped tw:text-muted-foreground so the reason inherits the tooltip's own foreground; italics kept; pbr rebuilt.)

Author response: the author fixed three of the four during the interview and kept Ian's tooltip wording on purpose.

Minor — Consider

  • M1: BiblicaLicensing.IsOnRestrictedList was public, and its doc described picker callers that don't exist. (fixed during review: made internal and reworded the doc; the tests reach it through InternalsVisibleTo.)
  • M2: The banner's messageFormatOf duplicated the choice of string key per notice kind in formatCopyrightNoticeMessage. (fixed during review: one exported getCopyrightNoticeMessageFormat helper is used by both. A deliberate break of the re-measure dependency makes its test fail.)
  • M3: The key %resourcePicker_restrictedModelText_tooltip% was also used in Manage Books, which is not a resource picker, and it is a screen-reader description as well as a tooltip. (fixed during review: renamed to %restrictedModelOrBaseText_disabledReason% in en and es and at every call site. The constant was renamed to match. A repo-wide grep for the old key is empty.)
  • M3 (other part): the flag has three names, isRestrictedAsModelText, isRestrictedAsBase and disableRestrictedModelTexts. (Author kept them: each names the role in its own context, a resource row "as a model text" and a project "as a base". Unifying them would mean renaming across C# and TypeScript for little gain.)
  • M4: The ResourcePickerDialog WithDisabledRows story disabled ESV and NLT, which aren't Biblica texts, with a hardcoded copy of the Biblica reason. (fixed during review: it now uses the sample reason "Not available for this translation", documented as consumer-supplied.)
  • M5: In the copyright-notice folder, BIBLICA_PERMISSIONS_URL lived in the message utils rather than the folder's const file, and the folder mixed .util and .utils file names. (fixed during review: moved the URL to copyright-notice.const.ts, and renamed open-in-browser.util.ts to open-in-browser.utils.ts with git mv.)
  • M6: In ProjectRowView, hasDisabledReason repeated a term of hasExtraTooltipContent. (fixed during review: hasDisabledReason is declared first and reused.)
  • M7: createModelTextRestrictionsCache used this, so destructuring sync or getWithin would break. (fixed during review: they are now closure-local functions. A new test destructures and calls them; it failed before the change.)
  • M8: Enhanced Resources already has its own copyright ribbon and dialog, so this is a second, parallel copyright UI. (Author: for information only. Consolidating would mean moving both into platform-bible-react, which is out of scope; it's a follow-up if a third surface appears.)
  • M9: The pickers treat restricted texts differently. The resource picker shows a lock icon, the row is reachable by keyboard and the tooltip opens on focus. Manage Books' "Based on" ProjectSelector row has no icon and a hover-only tooltip, and cmdk skips disabled items with the keyboard. (Author: fixing it means changing how the shared ProjectSelector handles disabled rows for every consumer. Follow-up.)
  • M10: The narrow-cell indicator's tooltip shows the full banner, about 50 words. (Author kept it: in a narrow Text Collection cell the tooltip is the only place the banner wording appears. The dialog shows Biblica's terms, a different text, so shortening it would drop part of the required notice.)
  • M11: Once a pane's banner is dismissed, the only way back to it is to switch texts. (Author: deliberate Paratext 9 parity. Switching to another text and back shows the banner again. Adding a "see it again" control would invent a requirement.)
  • M12: ResourceRowCells and DisabledResourceRow used physical-direction classes. (fixed during review: switched to pe-*, ps-* and text-end.)
  • M13: The details text says "Outside of Paratext" rather than a product name from the Product Names guideline. (fixed during review: it is Biblica's own required wording. A comment where the key is used says not to reword it without Biblica's agreement, and it is noted here.)
  • M14: No story opened the details dialog for a "Notification:" text, the one-paragraph-per-line case. (fixed during review: added a NotificationDetailsOpen story.)
  • M15: The platform-get-resources main.ts wiring was untested, both the "resync when a flag changed" decision and the 2-second wait-then-stamp. (fixed during review: extracted into syncModelTextRestrictionsAfterFlagSync, applyModelTextRestrictionsWithin and applyModelTextRestrictionsToCatalogWithin in model-text-restrictions.utils.ts, with 7 new tests, each of which fails when its code is deliberately broken. main.ts now just calls them.)
  • M16: The branch also adds a second feature, disabling restricted texts in the model and base pickers, with new public API. (addressed in this description: see the Overview and API Changes.)
  • M17: getCachedResources and getLocalNonDblResources can wait up to 2 seconds for the first restrictions fetch, even for callers that don't read the flag. (Author kept it: the wait only happens until the first fetch lands, a local C# call that normally takes milliseconds, and 2 seconds is the ceiling. Without it, a picker opened early could offer a restricted text; making it opt-in would add API for a startup-only edge case.)
  • M18: Disabled ProjectSelector rows now take pointer events, so a disabled, bound-but-closed row's "Open" button would become clickable. (fixed during review: the Open button is not rendered on a disabled row. A new test failed before the fix.)

Author response: the author asked for the reviewer's recommendation on each item, accepted fixing M1–M7, M12–M16 and M18, including the key rename in M3, and chose to skip M8–M11 and M17 for the reasons given.

Template Propagation

Shared Regions Modified

None.

Extension Config Changes

  • [n/a] extensions/src/platform-scripture/contributions/localizedStrings.json: extension-specific, no propagation needed
  • [n/a] extensions/src/platform-scripture/contributions/projectSettings.json: extension-specific, no propagation needed

Positive Observations

  • Additive API. All API additions are optional or additive: a new optional prop, an optional field, a new method, a new setting and new types. No existing export was removed, narrowed or reordered. The types used by the new signatures are exported where extension developers can reach them.
  • The setting follows an existing pattern. It follows PB_IS_PUBLISHED exactly: a get branch, a rejected set, a ProjectSettingsNames constant and a hidden projectSettings.json contribution. The C# wire shape is pinned by CopyrightNotice_SerializesToTheShapeTheFrontEndReads. CopyrightNotice tells consumers to treat an unknown kind as none, so the union can grow safely.
  • Restrictions fail safe. A missing localized string falls back to the key, so a restricted row stays disabled. A failed refresh keeps the last known restrictions rather than clearing them.
  • Hidden tabs are handled. At the measuring site, a comment explains that the banner's ResizeObserver catches up when the tab is shown, as .claude/rules/cross-view-sync-hidden-views.md requires. A test pins it.
  • Accessibility and RTL:
    • Names are isolated in <bdi>, and the permissions URL is forced left-to-right.
    • Copyright paragraphs use dir="auto".
    • The external link has a hidden "opens in browser" description.
    • The details dialog replaces the unlocalized corner ✕ with a localized Close, and focus returns to "More info" when it closes.
    • The resource picker's disabled rows stay reachable and pass their reason to screen readers.
  • Layout. The banner matches its neighbour SyncBlockedBanner and sits outside the scrolling area. It clamps to two lines, offering Show more only when the text overflows. Narrow Text Collection cells get an indicator instead.
  • Localization. Every string is localized in all 33 platform-scripture locales. copyright-notice-strings.test.ts checks that each locale has every key with the same placeholders.
  • Tests at every layer:
    • C#: licensing, backed by a survey of 1,819 DBL texts; notice and serialization; ListModelTextRestrictions; IsRestrictedAsBase.
    • Front end: the restrictions cache; the banner, indicator and panels; the resource picker, Team Layout and Manage Books wiring; guards that keep clicks and keys from a portalled dialog out of the Text Collection grid.
  • Reuse. The refactors shared code rather than copying it: resolveModelTextProjectId is now shared by the model-text panel and its web view, and ResourceRowCells avoids duplicating the picker row markup.
  • Housekeeping:
    • Comments are forward-facing.
    • The ADR entry, adr-biblica-license-notice-computed-setting, is in byte-order slug position and records the rejected alternatives.
    • No shadcn-ui files were modified.
    • The keyboard shortcuts catalog is still accurate.

Interview Notes

Purpose (author): "Show a banner on traditionally published Biblica texts explaining the copyright information."

Design explanation (Step 3.3): The author explained that Matt T provided the list of resources that must show the Biblica notice, and that the list was backed up programmatically by checking each resource's copyright; a text on the list gets the banner. The assistant filled in the code-level detail and the author accepted it:

  • Only resources are checked.
  • A resource on the list always gets the notice.
  • The copyright rule also catches Biblica texts not on the list, such as texts added to the DBL later. A few Open texts are exempted by id.
  • The same check drives the banner and the picker restrictions.

The copyright rule matched exactly the 55 listed texts among the 1,819 surveyed.

Scope decision, Paratext 9 parity: The author first expected the Biblica notice to show on editable projects too. After discussing it, they agreed to keep Paratext 9's behaviour:

  • A "Notification:" copyright banner shows on any project.
  • The Biblica restricted-license notice shows on resources only, because an editable project is the user's own translation.

The TSDoc was corrected to say exactly this.

Deliberate choices for the reviewer:

  • Dismissal matches Paratext 9. A dismissed banner stays dismissed for that pane until the pane switches to another text and back.
  • No end-to-end test. A DBL resource can't exist in an isolated e2e profile, so the e2e test was deleted. Unit and component tests cover the behaviour.
  • UI-only enforcement. The model/base restriction is enforced only in the pickers. An existing selection is kept and shows the banner.
  • Manage Books "Create based on" is in scope. It uses ProjectSummary.IsRestrictedAsBase.
  • Keyboard gap. cmdk skips disabled items with the keyboard, so Manage Books' reason is reachable by pointer hover only (M9, follow-up).
  • Translations. The notice strings were translated by AI into all 33 platform-scripture locales without native-speaker review, at the author's direction. The core key %restrictedModelOrBaseText_disabledReason% exists in en and es only, like the other resourcePicker_* keys.
  • The tooltip keeps Ian's ticket wording.
  • Biblica's wording is verbatim. "Outside of Paratext" in the details text is Biblica's required license text.
  • Follow-up for a shared copyright UI. Enhanced Resources has its own copyright ribbon and dialog (M8).
  • The details link opens through platform.openWindow. The web views' allowPopups was deliberately not widened.

Unresolved items: none. The author could explain every finding.

In-Review Quality Check

Every check passes, and none of them needed a fix.

  • npm run typecheck: pass.
  • ESLint: 0 errors across all changed files, per workspace. The full npm run lint takes over 40 minutes locally, so it was scoped to the changed files.
  • Prettier --check: clean on all changed files.
  • CSharpier: no changes.
  • npm test: pass.
    • Core: 5,915 passed.
    • Workspaces: all pass, including platform-scripture-editor (2,093), platform-scripture (647) and platform-get-resources (133).
    • platform-bible-react: 1,092 unit tests passed, plus the changed stories.
    • The full run hit some timeouts under CPU load. Each of those files passed when rerun alone, and the ones outside this branch were web-view.component.test.tsx, comment-list.stories and comment-thread.stories.
  • dotnet test c-sharp-tests/: 2,401 passed, 0 failed.

Rebased onto main (bbd52be5ac3, 16 new commits). Nine files conflicted, and main's changes were kept in each:

  • The resource picker's truncating cells and fixed column widths.
  • Team Layout's new picker dialog.
  • ProjectSelector's indicator tooltips.
  • The resource and model text panels' zoom area. The banner sits above it, so it stays at interface scale.
  • Manage Books' fullName.

Two of main's tests needed updating for this branch: one ProjectSelector test's query, and a source-contract test that now pins the banner between the resource selector and the zoom area.

After the rebase:

  • Checks: typecheck and the C# licensing/restriction tests (681) pass, and lint and Prettier are clean on the resolved files.
  • Tests:
    • Dialogs: 163.
    • Picker and selector: 217, with the Open-button test re-checked by removing its fix.
    • Manage Books and platform-get-resources: 191.
    • platform-scripture-editor: 2,133.

Suggested Review Focus

  • BiblicaLicensing.IsRestrictedLicense:

    • the list-then-copyright-rule order
    • the Open-text exemptions by id
    • whether the copyright rule should stay as a safety net or be removed in favour of the list alone

    The author explained the list at a product level; walk through the fallback rule together.

  • Paratext 9 parity scope. Is showing "Notification:" banners on editable projects, and the Biblica notice on resources only, the product decision we want?

  • Second feature: disabling restricted texts in the model and base pickers. Review its public API (getDisabledReason, disableRestrictedModelTexts, listModelTextRestrictions, isRestrictedAsModelText), and confirm that UI-only enforcement is acceptable.

  • ProjectSelector disabled rows now take pointer events. This is a shared component, so check other consumers.

  • The up-to-2-second first wait in getCachedResources and getLocalNonDblResources (M17).

  • Follow-ups to file. The Manage Books keyboard reachability for disabled rows (M9), and a shared copyright UI in platform-bible-react (M8).

AI-assisted — Claude Code (Claude Opus 5.5)

🤖 Generated with Claude Code


This change is Reviewable

@katherinejensen00
katherinejensen00 force-pushed the pt-4731-biblica-texts-special-message branch 2 times, most recently from 91e8947 to de1fb36 Compare September 28, 2026 14:42
katherinejensen00 and others added 6 commits September 29, 2026 10:24
Show a dismissible licence banner on Biblica's traditionally licensed
(non-Open) texts, and keep them from being picked as a model or base text.

Detection (C#)
- BiblicaLicensing: Biblica's list of 56 DBL ids (rights holder Biblica,
  not open access) is the authority; a copyright rule ("Biblica, Inc" /
  "Biblica®", not "Biblica® Open", OBTT exempted by id) also counts, so an
  installed Biblica text added to the DBL later is covered. A survey of all
  1,819 resources in Platform.Bible's list shows the rule and the list agree.
- New read-only computed project setting platformScripture.copyrightNotice:
  none | notification (PT9's "Notification:" copyright banner, e.g. ESV) |
  restrictedLicense (with the years of the text's copyright statement).
- DblResourceData.IsRestrictedAsModelText for catalog rows;
  ProjectSummary.IsRestrictedAsBase for Manage Books.

Banner (platform-scripture-editor)
- CopyrightNoticeBanner in Simple Column 1 and Column 3 and the Power-mode
  editor: bold name, clamped to two lines with Show more (re-measured when
  a hidden tab becomes visible), More info… opens Biblica's terms with a
  link to biblica.com/permissions, dismissal kept per pane and per text.
- Text Collection cells get an info icon carrying the notice.
- Strings live in platform-scripture and are translated into all 33 of its
  locales (AI drafts).

Pickers
- ResourcePickerDialog gains getDisabledReason; the Column 1 model text
  picker and Team layout's model text picker grey out restricted texts
  with a tooltip. Already-selected ones can still be deselected.
- Manage Books "Create based on" shows restricted texts but not as choices.

Also: Storybook stories for the banner and the picker's disabled rows,
rebuilt platform-bible-react/-utils dist and papi.d.ts, and
adr-biblica-license-notice-computed-setting.

Enhanced Resources is not covered: an Enhanced Resource is not a ScrText
project, so the setting cannot reach it.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Restriction
- One check, BiblicaLicensing.IsRestricted(scrText), used by the notice
  setting and Manage Books. It applies only to resources, so a Biblica
  team's own editable project (or Biblica's master project, whose DBL id
  matches the list) is never flagged. It reads the typed, entity-decoded
  plain-text copyright, survives a malformed DBL id, and matches more
  spellings (Biblica after a non-Latin letter, Arabic and full-width
  commas, "Biblica Open" without ®).
- The notice now carries its names (name, fullName falling back to the
  short name), so panes read one setting and a missing full name no
  longer shows "*Name Missing*". It is built through factories, and
  CopyrightNoticeKind has its own file.
- Pickers no longer rely on a flag baked into the cached DBL catalog: a
  new listModelTextRestrictions call (no network needed) returns Biblica's
  ids and the installed restricted resources, and platform-get-resources
  stamps isRestrictedAsModelText on every catalog and local row it serves.
  This covers an old persisted cache, offline sessions, local rows and
  Biblica texts added to the DBL after the list was made.

Banner and details
- Dismissal matches Paratext 9: a pane remembers only the text whose
  notice was dismissed, so switching to another text and back shows it
  again. The banner and grid icon are keyed by project, so a switch never
  shows the previous text's notice.
- The details dialog is modal, returns focus to its opener, has a working
  width and a localized Close, and announces its text. The permissions
  URL is a real link, opened in the browser through platform.openWindow.
- role="note" instead of a live region; the dismiss button has a tooltip
  and a clearer label; the grid icon has a short label and its tooltip is
  one paragraph; "Show more" re-measures when the wording changes.
- Text Collection: clicks, keys, right-clicks and Escape inside the
  details dialog no longer reach the verse row.
- Model texts stored as project references get their notice too.
- The Power/Simple editor asks every project, since a "Notification:"
  copyright shows its banner on any text, as in Paratext 9.

Pickers
- ResourcePickerDialog disabled rows: reachable like other rows, name plus
  reason as description, lock icon, no hover highlight, no selection by
  click, Enter or Space; shared row cells extracted.
- ProjectSelector disabled rows keep pointer events so their reason
  tooltip opens, and carry the reason as their description.
- The picker tooltip is a core string (%resourcePicker_restrictedModelText_tooltip%).

Also: notice strings sorted in every locale; wiring tests for the web
views, Team layout, Manage Books and the restriction list; stories for the
grid icon, grid cells and the model text panel; the Storybook stub no
longer crashes the grid stories; rebuilt platform-bible-react/-utils dist;
ADR adr-biblica-license-notice-computed-setting updated to the final
design.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Correct the CopyrightNotice and copyrightNotice setting TSDoc:
  restrictedLicense applies only to resources, notification to any
  project whose copyright starts with "Notification:"
- Drop the ellipsis from "More info" in all locales; it opens an
  informational dialog
- Give the ProjectSelector disabled reason the tooltip's own foreground
  so it meets contrast on the inverted tooltip background
- Rename the disabled-reason key to %restrictedModelOrBaseText_disabledReason%,
  since Manage Books uses it too
- Hide the bound-but-closed Open button on disabled ProjectSelector rows
- Make BiblicaLicensing.IsOnRestrictedList internal
- Share one helper for the notice's message format between the banner and
  the message utils
- Move BIBLICA_PERMISSIONS_URL to the const file; name utils files .utils
- Note that the details wording is Biblica's own and not to be reworded
- Add a story for the Notification details dialog
- Make the model-text restrictions cache free of `this`, and extract the
  resync decision and bounded wait-then-stamp from platform-get-resources'
  main.ts into tested utils
- Use a sample reason in the ResourcePickerDialog disabled-rows story and
  logical-direction classes in its row cells

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The resource tests read the notice through the project data provider,
which refuses to open a resource without a valid Paratext registration.
CI machines have none, so all eight failed there while passing on
registered developer machines. Ask CopyrightNotice.FromScrText directly
for the resource cases; the editable-project tests still cover the
provider routing the setting.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CopyrightNotice.fullName already falls back to the short name on the
C# side, as its type documents, so the dialog's `fullName || name`
repeated that guarantee and tripped the project-name adoption sweep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rebased onto main after PT-4585 (#2849) moved the zoom markers inside the
editor scroll boxes and reworked the resource cell's right-click menu:

- Portalled right-click test and copyright stories use the new cell props
  (required zoomArea, menuStrings fixture, Copy-only menu)
- Grid test's verse-text ids no longer match the /^cell-/ cell count
- Rebuilt platform-bible-react and platform-bible-utils dist

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@katherinejensen00
katherinejensen00 force-pushed the pt-4731-biblica-texts-special-message branch from de1fb36 to 5d0046e Compare September 29, 2026 19:01

@lyonsil lyonsil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the Biblica license notice and the model-text restrictions: 11 findings. Each comment shows the severity it settled at and a status; checked and confirmed means the finding was verified against the code at this head, and needs a human call means it could not be settled either way and is yours to judge. Findings that do not sit on a line this PR changed are listed below instead of inline.

Findings that could not be attached to a line in this diff

#9 (low · checked and confirmed) extensions/src/platform-scripture-editor/src/scripture-text-grid/resource-cell-view.component.tsx:393 - Drag-selecting text inside a cell's copyright details window runs the Text Collection grid's column-drag start handler, recording that column as the one being dragged although no column is being moved.

What happens: the copyright details dialog is portalled to document.body, but React still bubbles its synthetic events up the component tree. In chapter context copyrightIndicator renders inside the draggable data-cell-header div (resource-cell-view.component.tsx:424), whose onDragStart is headerDrag?.onDragStart with no containment check (:394). A text drag inside the dialog therefore calls headerDrag.onDragStart, which sets draggedIdRef (scripture-text-grid.component.tsx:220-222).

Why it matters: the dialog's dragend also bubbles to onDragEnd (:395) and clears that state, so nothing moves today; the cost is a reorder handler running on events that are not its own. The sibling contextmenu handler in this file already carries the guard for the same reason (:335). Only reachable when a restricted or Notification text sits in a reorderable chapter-view grid.

Fix: In resource-cell-view.component.tsx, replace onDragStart={headerDrag?.onDragStart} on the data-cell-header div (:394) with a handler that calls headerDrag.onDragStart() only when event.target instanceof Node && event.currentTarget.contains(event.target) (the guard handleCellContextMenu uses at :335), and leave it undefined when headerDrag is. Add a test in resource-cell-view.component.test.tsx next to the existing portalled-window guard test that fires a dragstart from inside a portalled child of copyrightIndicator and asserts headerDrag.onDragStart is not called (and is called for a drag on the header itself). The change must be forward-facing in any comment it adds.

How this was checked: The copyright details dialog is a Radix Dialog portalled to document.body (dialog.tsx:34-40, 128), so React bubbles its drag events through the component tree. In the chapter-context branch the indicator is a child of the data-cell-header div (resource-cell-view.component.tsx:424), whose onDragStart={headerDrag?.onDragStart} (:394) has no containment check, so a text drag in the dialog runs the grid's onDragStart and sets draggedIdRef (scripture-text-grid.component.tsx:220-222). The effect is small: the dragend from the dialog also bubbles to onDragEnd (:395), which clears the ref, and the drop-target ring is suppressed for the dragged column itself (:218-219). The same PR already guards the sibling contextmenu handler at :335 for this reason, and this PR introduced the indicator that makes the path reachable.

Not attached to a line: not on a changed line; cannot be posted inline

#10 (low · checked and confirmed) extensions/src/platform-scripture-editor/src/scripture-text-grid/scripture-text-grid.component.tsx:433 - Drag-selecting text inside a cell's copyright details window runs the Text Collection grid's reorder drag handlers, which record that cell as the one being dragged and accept drags over it, although no cell is being moved.

What happens: the copyright details dialog is portalled to document.body, but React still bubbles its synthetic events up the component tree. On the draggable listitem only onClick has a containment guard (scripture-text-grid.component.tsx:413-420); onDragStart, onDragOver and onDrop are wired straight to reorder (:433-437), and the ResourceColumn wrapper wires onDragOver and onDrop the same way (resource-column.component.tsx:73-74). A text drag inside the dialog sets draggedIdRef (scripture-text-grid.component.tsx:220-222), and onDragOver calls preventDefault and setDragOverId (:226-229).

Why it matters: no drop ring shows, because isDropTarget excludes the dragged cell (:218-219), and the dialog's dragend clears the state (:223-226), so nothing moves today; the cost is reorder handlers running on events that are not their own, on an element whose click handler already guards against exactly this. Only reachable when a restricted or Notification text sits in a reorderable grid.

Fix: Wrap reorder.onDragStart, reorder.onDragOver and reorder.onDrop at both scripture-text-grid.component.tsx:433-437 and resource-column.component.tsx:73-74 so each runs only when event.target instanceof Node && event.currentTarget.contains(event.target), matching the guard the listitem's onClick uses at :413-420; leave onDragEnd as is. Add tests in scripture-text-grid.component.test.tsx (next to the existing click-guard test) and in the resource-column tests that fire dragstart/dragover/drop from a portalled child and assert no reorder state change or onReorder call, while a drag on the cell itself still reorders.

How this was checked: In the verse-row listitem, onDragStart, onDragOver and onDrop are wired directly to reorder (scripture-text-grid.component.tsx:433-437) with no containment check, while this PR added one only to onClick (:413-420) and a target check to onKeyDown. The dialog is portalled but its synthetic drag events bubble to this listitem, so a text drag in the dialog sets draggedIdRef (:220-222), onDragOver calls preventDefault and setDragOverId (:226-229). The ring is not shown: isDropTarget excludes the dragged resource (:218-219), and the dialog sits over the grid so only the owning cell receives the events; dragend bubbles to onDragEnd (:223-226) and clears state. The same unguarded onDragOver/onDrop also sit on the ResourceColumn wrapper (resource-column.component.tsx:73-74), which the dialog bubbles into for chapter and aligned views.

Not attached to a line: not on a changed line; cannot be posted inline

#11 (low · checked and confirmed) src/shared/data/keyboard-shortcuts.data.ts:539 - The keyboard shortcuts catalog still describes the Text Collection's Enter/Space and Escape shortcuts without the conditions this change added to when they fire.

What happens: Enter/Space now activate a cell only when the listitem itself has focus (scripture-text-grid.component.tsx:425), and Escape now ignores presses inside an open dialog, decided in the new chapter-context-escape.utils.ts. The catalog entries scripture-text-grid-open-chapter-context and scripture-text-grid-close-chapter-context (keyboard-shortcuts.data.ts:539-566) were not touched, and the Escape entry's locations does not list the new utils file.

Why it matters: documentation only, with no behavior effect; .claude/rules/keyboard-shortcuts-catalog.md asks that each changed handler's context and locations stay accurate.

Fix: update the context text of both entries, and add extensions/src/platform-scripture-editor/src/scripture-text-grid/chapter-context-escape.utils.ts to the Escape entry's locations.

How this was checked: The PR diff does not touch src/shared/data/keyboard-shortcuts.data.ts. The Enter/Space handler in scripture-text-grid.component.tsx:421-431 now returns early unless event.target === event.currentTarget, so a control inside the row keeps its own Enter/Space. The Escape decision moved into the new chapter-context-escape.utils.ts, imported at scripture-text-grid.web-view.tsx:70, and ignores presses inside [role="dialog"]/[role="alertdialog"]. The catalog entries at keyboard-shortcuts.data.ts:539-547 and 560-566 still say only "Scripture Text Grid web view", and the Escape entry lists only the web-view file. Documentation only, no behavior effect.

Not attached to a line: not on a changed line; cannot be posted inline

(AI-assisted, with my guidance)

ProjectSettingsNames.PT_VERSIFICATION,
"4"
),
IsRestrictedAsBase: BiblicaLicensing.IsRestricted(scrText)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3 - low · checked and confirmed

One resource whose copyright or full-name setting throws when read makes the whole Manage Books project list fail to load.

What happens: ProjectSummary.FromScrText now calls BiblicaLicensing.IsRestricted (ProjectSummary.cs:112), which reads Settings.CopyrightPlainText through GetPlainTextCopyright (BiblicaLicensing.cs:152) and Settings.FullName without a guard; only ReadDblId catches (:162). ProjectFilterService.ToProjectListResult runs Select(ProjectSummary.FromScrText) over every project (ProjectFilterService.cs:131), so one exception fails the whole request. The comment in this factory says its lookups are kept cheap and exception-free, and ListModelTextRestrictions wraps the same call per project for this reason (DblDownloadableDataProvider.cs:176-188).

Why it matters: rare, but when it happens Create, Copy and Delete in Manage Books are all unavailable.

Fix: in ProjectSummary.FromScrText, call BiblicaLicensing.IsRestricted through a guard that logs and returns false on an exception, matching ListModelTextRestrictions, and add a ProjectFilterServiceTests case with a resource whose settings throw.

How this was checked: ProjectSummary.FromScrText (c-sharp/ManageBooks/ProjectSummary.cs:112, added by this PR) calls BiblicaLicensing.IsRestricted, which for any resource reads Settings.CopyrightPlainText via GetPlainTextCopyright and Settings.FullName with no guard (BiblicaLicensing.cs:141-153); only ReadDblId catches (:155-165), and its own comment says a resource's zipped DBL metadata can be unreadable. ProjectFilterService.ToProjectListResult maps every project through FromScrText with a plain Select (ProjectFilterService.cs:131), so one throw fails the whole list request. The sibling ListModelTextRestrictions wraps the same call per project in try/catch for exactly this reason (DblDownloadableDataProvider.cs:176-188). This was traced in the code, not reproduced with a resource whose copyright read actually throws, so how often it happens is unknown.

private static partial Regex BiblicaOpen();

/// <summary>Four decimal digits in any script</summary>
[GeneratedRegex(@"\d{4}")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4 - low · checked and confirmed

The restricted-license details can quote a wrong copyright year when the copyright statement contains a number of five or more digits.

What happens: FourDigits is \d{4} with no digit boundaries (BiblicaLicensing.cs:127-128). GetCopyrightYears scans the text after each © (:215-226), so a run such as 20112 or an ISBN fragment yields a four-digit slice in the 1900-2099 range, which is kept as a year (:230-231).

Why it matters: the years are inserted into Biblica's required legal wording (%platformScripture_copyrightNotice_restrictedLicense_details%). The surveyed texts pass today, so this only bites on a new or updated copyright statement.

Fix: Change the FourDigits pattern at BiblicaLicensing.cs:127 to (?<!\d)\d{4}(?!\d), so only a run of exactly four digits is read as a year. Add a BiblicaLicensingTests case that fails without the change, for example GetCopyrightYears("© Biblica Inc. 20112, 1984") returning "1984", and update the FourDigits summary to say it matches exactly four digits.

How this was checked: The FourDigits regex at c-sharp/Projects/DigitalBibleLibrary/BiblicaLicensing.cs:127 is the bare pattern \d{4} with no digit boundaries. GetCopyrightYears (:209-237) runs it over the text from the first © onward, and Matches yields the first four digits of any longer run, so "© 20112" gives the year 2011 and "© 19781984" gives 1978 and 1984. The 1900-2099 range check at :230 does not reject these slices. The whole file is new in this PR. Existing tests (BiblicaLicensingTests.cs:214-253) have no case with a run of five or more digits, so nothing pins the behavior. It only bites on a copyright statement containing such a run, and the surveyed texts pass.

});
}

async function getCachedResources(): Promise<DblResourceCatalog> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1 - medium · checked and confirmed

A resource picker or Team Layout opened in the first seconds after startup can offer restricted Biblica texts as selectable model texts, and they stay selectable until that dialog is closed and reopened.

What happens: getCachedResources (main.ts:386-394) waits at most MODEL_TEXT_RESTRICTIONS_WAIT_MS (2000, main.ts:51) for the first restrictions fetch. When getWithin times out it resolves undefined (model-text-restrictions.utils.ts:126-137), and applyModelTextRestrictions returns the rows unstamped (:16-18). resource-picker.dialog.tsx:62 and team-layout.dialog.tsx:122 fetch the catalog once through useRetryablePromise, so getRestrictedModelTextReason sees isRestrictedAsModelText as undefined for the rest of that dialog's life.

Why it matters: the pickers fail open whenever the C# DBL provider takes longer than 2s to answer, which is most likely on a cold start, and the license restriction is the point of this feature.

Fix: A row whose restriction status is still unknown must not be selectable, and the dialog must pick up the status once it is known. In getCachedResources (main.ts:386-394), mark the available result as restrictions-pending when getWithin returns undefined (add an optional field to the available arm of DblResourceCatalog in types/platform-get-resources.d.ts:153, with TSDoc, and set it in applyModelTextRestrictionsToCatalog, model-text-restrictions.utils.ts:41-48). In resource-picker.dialog.tsx (:62) and team-layout.dialog.tsx (:122), while that flag is set, disable restricted-candidate rows (fail closed: report a pending reason from getRestrictedModelTextReason, restricted-model-text.utils.ts:19) and call the existing refetch from useRetryablePromise at a bounded delay until the flag clears. Both dialogs must change, since they fetch independently. Add a test in each dialog's test file (resource-picker.dialog.test.tsx, team-layout.dialog.test.tsx) that serves a pending catalog, then a stamped one, and asserts the restricted row is unselectable before and after the refetch as appropriate, plus an extension-side test that getCachedResources flags a timed-out read.

How this was checked: getCachedResources (main.ts:386-394) passes the 2000 ms bound (main.ts:51, added by this PR) to applyModelTextRestrictionsToCatalogWithin, which uses cache.getWithin (model-text-restrictions.utils.ts:126-137). On timeout getWithin resolves undefined and applyModelTextRestrictions returns rows unstamped (:16-18), so isRestrictedAsModelText is undefined and getRestrictedModelTextReason (restricted-model-text.utils.ts:19-27) returns no reason, leaving the row selectable. Both resource-picker.dialog.tsx:62-70 and team-layout.dialog.tsx:118-125 call getCachedResources once through useRetryablePromise with no refetch trigger other than the error/notReady retry, and the unstamped catalog is a normal available result, so it is never refetched until the dialog is reopened. The wait includes papi.dataProviders.get for the C# provider (main.ts:200-203), which can exceed 2 s while the provider registers; a failed fetch (undefined) produces the same fail-open. The existing test at model-text-restrictions.utils.test.ts:408-425 pins the unstamped-after-bound behavior but nothing covers the dialogs recovering.

Other findings in this file: #5, #6

async function getCachedResources(): Promise<DblResourceCatalog> {
// Waits, boundedly, for the first restrictions fetch so a picker opened early is not fail-open;
// after that the restrictions are in memory and this adds nothing.
return applyModelTextRestrictionsToCatalogWithin(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5 - low · checked and confirmed

Home, Get Resources and the Text Collection can take up to 2 seconds longer to load their resource lists at startup, waiting for a restriction flag they never read.

What happens: every platformGetResources.getCachedResources call now goes through applyModelTextRestrictionsToCatalogWithin with a 2000 ms wait (main.ts:389-393), whoever the caller is: home.web-view.tsx:70, get-resources.web-view.tsx:39 and scripture-text-grid.web-view.tsx:263 all reach it, and none of them reads isRestrictedAsModelText.

Why it matters: startup latency on the most common screens, paid only while the first restrictions fetch is outstanding.

Fix: Bound the wait only for callers that read isRestrictedAsModelText. Recommended: add an optional argument to 'platformGetResources.getCachedResources' (platform-get-resources.d.ts:219, with TSDoc) and to its handler in main.ts:386, so the 2000 ms applyModelTextRestrictionsToCatalogWithin wait runs only when it is set; pass it from resource-picker.dialog.tsx:62 and team-layout.dialog.tsx:122, and leave home.web-view.tsx, get-resources.web-view.tsx, scripture-text-grid.web-view.tsx and use-dbl-resource-catalog.hook.ts calling it bare (those rows are then returned without waiting, restrictions are still applied once known). Regenerate lib/papi-dts/papi.d.ts with npm run build:types, and add a test that a bare getCachedResources call resolves before a never-settling restrictions fetch while the opted-in call still waits (it must fail if the opt-in is removed).

How this was checked: getCachedResources (main.ts:386-394) wraps every call in applyModelTextRestrictionsToCatalogWithin(..., 2000), which runs restrictionsCache.getWithin alongside the catalog read in a Promise.all (model-text-restrictions.utils.ts:180-190). getWithin returns at once only when known is set; otherwise it races the first listModelTextRestrictions call (a lazy fetch via papi.dataProviders.get('platformGetResources.dblResourcesProvider'), main.ts:200-205) against the 2000 ms timeout. So until the first fetch lands, even a cache-hit catalog read is held back by up to 2 s. home.web-view.tsx:70, get-resources.web-view.tsx:39, scripture-text-grid.web-view.tsx:263 and use-dbl-resource-catalog.hook.ts:94 all call it with no argument, and none reads isRestrictedAsModelText. The only consumers of the flag are the resource picker (model-text-panel.web-view.tsx:172, which passes disableRestrictedModelTexts: true) and team-layout.dialog.tsx:105-107. The cost is limited to the first call(s) while the provider is slow, then it is free.

Other findings in this file: #1, #6


const allMetadata = await getLocalProjectMetadata();
return buildLocalNonDblResources(allMetadata, dblCatalog);
return await applyModelTextRestrictionsWithin(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6 - low · checked and confirmed

At startup, the resource picker's Installed list of non-DBL resources waits as long as the model-text restrictions fetch takes, not the intended 2 seconds at most.

What happens: getLocalNonDblResources waits for the model-text restrictions three times in a row. getCachedResources (main.ts:409) waits up to MODEL_TEXT_RESTRICTIONS_WAIT_MS (2000). ensureInstalledFlagsSynced (main.ts:412) then runs syncModelTextRestrictionsAfterFlagSync (main.ts:307), whose sync(false) is ensureLoaded: it awaits the fetch already in flight, or starts a new one after a failure, with no time limit (model-text-restrictions.utils.ts:112-115). Only after that does applyModelTextRestrictionsWithin (main.ts:420-424) apply its own 2000 ms bound.

Why it matters: the 2-second bound does not limit this list. With a slow or failing backend at startup, the Installed section of the resource picker stays empty until the restrictions fetch settles.

Fix: Make getLocalNonDblResources wait for the model-text restrictions at most once, for MODEL_TEXT_RESTRICTIONS_WAIT_MS in total. Changing getCachedResources() at main.ts:409 to readCatalog() is not enough: ensureInstalledFlagsSynced() at main.ts:412 still awaits the restrictions fetch with no bound through syncModelTextRestrictionsAfterFlagSync (main.ts:307). Keep that unbounded restrictions step out of the awaited path here (for example by awaiting only the flag sync and leaving the restrictions sync in the background), and keep the single bounded applyModelTextRestrictionsWithin call at main.ts:420-424 so rows are still stamped. Add a test in extensions/src/platform-get-resources/ with a never-resolving restrictions fetch asserting the list resolves within about MODEL_TEXT_RESTRICTIONS_WAIT_MS; it must fail without the change.

How this was checked: getLocalNonDblResources waits on the restrictions three times in sequence: getCachedResources bounded to 2s (main.ts:409, 386-393), then ensureInstalledFlagsSynced (main.ts:412), whose sync step awaits the restrictions fetch with no bound (main.ts:307-310 via sync/ensureLoaded in model-text-restrictions.utils.ts:111-124), then applyModelTextRestrictionsWithin with a further 2s bound (main.ts:420-424). If the first fetch is still outstanding, the 2s bound is moot because the sync waits for it to finish; if it failed, getWithin starts a retry (utils.ts:126-137) and adds up to 2s more. The wait is new in this PR (the 420-424 call and the restrictions step in the sync). It only affects the startup window with a slow or failing backend.

Other findings in this file: #1, #5

* @param flagSync `isRefreshRequested`: whether the sync was asked to refresh everything it can;
* `isAnyFlagChanged`: whether it changed any resource's flags.
*/
export function syncModelTextRestrictionsAfterFlagSync(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2 - medium · checked and confirmed

A restricted Biblica text added mid-session outside the DBL catalog (copied in by hand, or installed by Paratext 9) stays selectable as a model text until the next install, uninstall or resource-flag refresh.

What happens: such a resource becomes a local non-DBL row whose dblEntryUid is its own projectId, so only the restrictions' projectIds can match it (model-text-restrictions.utils.ts:28-29). platform.onDidChangeProjects calls syncAfterInFlight(false) (main.ts:706), and syncFlags reports isAnyFlagChanged only when a catalog row's flags change (main.ts:271-272). syncModelTextRestrictionsAfterFlagSync therefore calls sync(false) (:155), which is ensureLoaded and returns the cached restrictions without asking the backend again, so projectIds never gains the new project.

Why it matters: the restriction goes unenforced for exactly the texts that only the backend's copyright rule can catch.

Fix: In the platform.onDidChangeProjects listener in extensions/src/platform-get-resources/src/main.ts (:700-707), refresh the restrictions as well as the flags. Call modelTextRestrictions.refresh() after syncAfterInFlight(false) has settled, so a sync that already started does not overwrite it with an older read. Do not switch the call to syncAfterInFlight(true), because that adds an updateAvailable backend round trip the adjacent comment excludes. Add a test that fails without the change: either extract the listener's body into a function in model-text-restrictions.utils.ts that can be exercised directly, or mount main.ts's activation with a mocked network event. The test should load the restrictions, then add a local row whose project id appears only in a second backend answer, fire the project-change event, and assert the row is stamped. If the comment above the listener documents sync cost, extend it to say the restrictions are refetched too.

How this was checked: The restrictions are matched for a local non-DBL row only through projectIds (model-text-restrictions.utils.ts:28-29), and the backend builds that list by scanning installed projects at call time (DblDownloadableDataProvider.cs:179-201). platform.onDidChangeProjects calls syncAfterInFlight(false) (main.ts:706). That sync reaches syncModelTextRestrictionsAfterFlagSync (main.ts:307), which calls sync(false) only when no refresh was requested and no catalog flag changed (utils.ts:155). sync(false) is ensureLoaded, which returns the cached value when it is known (utils.ts:112-115). A hand-copied non-catalog project has no catalog row, so syncFlags reports no change and the new project id never reaches the cache. getLocalNonDblResources then stamps from that stale cache via getWithin (main.ts:421-423, utils.ts:127). The stale list is replaced at the next install or uninstall that changes a flag, or the next platformGetResources.refreshResourceFlags call (main.ts:353, 621-623), each of which refetches.

function CopyrightNoticeDetails({ notice, localizedStrings }: CopyrightNoticeDetailsProps) {
const opensInBrowserId = useId();

if (notice.kind === 'notification')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#7 - low · checked and confirmed

For a "Notification:" copyright of a single paragraph, More info opens a window that shows only its title and a Close button.

What happens: CopyrightNotice.FromScrText builds details from the non-empty lines after the first (CopyrightNotice.cs:100-104), which is "" for a one-paragraph copyright. CopyrightNoticeDetails splits and filters that to no paragraphs (copyright-details-dialog.component.tsx:36-46). CopyrightNoticeBanner still renders the dialog with its More info trigger (copyright-notice-banner.component.tsx:102-110), and CopyrightNoticeIndicatorView still wraps its Info button in the dialog (copyright-notice-indicator.component.tsx:35-55). The banner test already builds such a notice (copyright-notice-banner.component.test.tsx:137) but never opens the dialog.

Why it matters: a dead-end window for any one-paragraph Notification copyright, in the banner and in a narrow Text Collection cell alike.

Fix: Add a shared predicate next to ShowableCopyrightNotice in copyright-notice-message.utils.tsx (for example hasCopyrightDetails(notice), true for restrictedLicense and for notification with non-empty notice.details) and use it at both sites. In CopyrightNoticeBanner (copyright-notice-banner.component.tsx:102-110), render the CopyrightDetailsDialog with its More info button only when it is true. In CopyrightNoticeIndicatorView (copyright-notice-indicator.component.tsx:35-55), when it is false render the Info Button directly inside TooltipTrigger with no dialog, keeping its aria-label and event.stopPropagation() onClick, so the tooltip still shows the notice. Add a banner test that a notification notice with details: '' shows no More info button, and an indicator test that the same notice shows the tooltip trigger but opens no dialog on click.

How this was checked: CopyrightNotice.FromScrText (c-sharp/Projects/CopyrightNotice.cs:96-111) builds Details from lines after the first, trimmed and non-empty, so a one-paragraph "Notification:" copyright yields "". CopyrightNoticeDetails (copyright-details-dialog.component.tsx:36-46) splits and filters that to zero paragraphs, so the dialog body is empty beneath the title (:107-112) and the Close footer. CopyrightNoticeBanner renders CopyrightDetailsDialog with its More info trigger unconditionally (copyright-notice-banner.component.tsx:102-110), and CopyrightNoticeIndicatorView wraps its button in the dialog unconditionally (copyright-notice-indicator.component.tsx:35-55). The empty-details notification is a real shape: the banner test already builds one with details '' (copyright-notice-banner.component.test.tsx:137) but never opens or asserts on the dialog. The indicator's tooltip shows the full notice sentence (indicator.component.tsx:56-58), so hiding its dialog would not hide the notice.

@@ -0,0 +1,11 @@
/** Windows Escape closes by itself, such as a cell's copyright details */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#8 - low · checked and confirmed

In the Text Collection's chapter view, pressing Escape in a cell's right-click menu or zoom dropdown closes the whole chapter view instead of just the menu.

What happens: the grid listens for keydown on window in the capture phase (scripture-text-grid.web-view.tsx:237), which runs before Radix's own Escape handling, and calls stopPropagation. isEscapeForChapterContext exempts only targets inside DIALOG_SELECTOR, [role="dialog"], [role="alertdialog"] (chapter-context-escape.utils.ts:2). Radix menus use role="menu", so Escape pressed in one reaches handleCloseChapterContext.

Why it matters: the gap predates this change, which rewrote this function to exempt dialogs; any menu opened from a chapter-view cell is still affected.

Fix: In chapter-context-escape.utils.ts, extend the selector (DIALOG_SELECTOR) to also match [role="menu"] and rename it (for example ESCAPE_DISMISSIBLE_LAYER_SELECTOR) so it names any open layer that Escape dismisses first; update the comment on the constant and the TSDoc on isEscapeForChapterContext to say "window or menu". Add a menu case to the it.each in chapter-context-escape.utils.test.ts asserting Escape pressed on an item inside a role="menu" element returns false (this test fails without the selector change). Keep the keydown handler at scripture-text-grid.web-view.tsx:229-238 unchanged.

How this was checked: The window-level capture keydown listener in scripture-text-grid.web-view.tsx:229-238 calls stopPropagation on any Escape that isEscapeForChapterContext (chapter-context-escape.utils.ts:2-11) does not exempt, and only [role="dialog"], [role="alertdialog"] targets are exempt. Each ResourceCell with a usable state renders a Radix DropdownMenu for the right-click menu (resource-cell-view.component.tsx:477-492) and another for the zoom "options" dropdown (:425-459). Radix gives both content elements role="menu" and moves focus into them, so Escape there is swallowed before Radix's document-level Escape handler (the in-code comment at resource-cell-view.component.tsx:498 says "Radix handles Escape on the document") and the chapter split closes instead. Reachable whenever a chapter split is open in verse view and a menu is opened from any cell (verse column or chapter pane). Before this PR every Escape closed the split; the PR added the dialog exemption but not menus, so the gap is not a regression. No Select/listbox is rendered in these cells, so only role="menu" is needed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants