Hide Share Layout button in power mode; polish dialog UI

[captaincrazybro]

Code Review Summary

Branch: fix-share-layout-in-power

Base: origin/main

Date: 2026-07-15

Review model: Claude Sonnet 5

Files changed: 5

Overview

This branch hides the "Share Layout with Team" toolbar button in the scripture editor when interface mode is power (it's a simple-mode-only concept — power mode's more complex and user-customizable layout with multiple scroll groups replace the single shared-layout concept it targets), reusing the file's existing isPowerMode conditional-render pattern. It also polishes the Share Layout dialog's UI: the "manage Bible/Commentary resources" card and the "Text collection resources" checklist card are merged into one card, the divider between individual resource entries and the divider directly under the "Text collection resources" label are removed, all five section labels ("Model text", "Default tab", "Project Bible texts", "Project Commentaries", "Text collection resources") are made bold and consistent, the "Bible texts" label is renamed to "Project Bible texts" (with "Project Commentaries" capitalized to match), the "Default tab" dropdown's grayed-out background is fixed, and entry text sizing/color is adjusted.

Screenshot of adjusted share layout dialog:

API Changes

None. All changed files are outside tracked public API surfaces (lib/platform-bible-react/, lib/platform-bible-utils/, papi.d.ts, extensions' src/types/*.d.ts). No exports were added, modified, or removed.

Findings

Critical — Must address before merge

• assets/localization/en.json: Modified the existing localization key %shareLayoutDialog_scriptureResources_label% in place ("Bible texts" → "Project Bible texts") instead of creating a new key and redirecting the old one via metadata.fallbackKey, per the CRITICAL "Existing Strings Are Immutable" rule in .context/standards/Localization-Guide.md:282-284,367 (a listed Blocking Issue). (Author: the key was introduced only 2 days ago in PT-4041/PT-4041: Add Share Layout with Team dialog for project admins #2528 — verified via git log -S that it was added 2026-07-13 — so there are no downstream consumers or shipped translations at risk yet; an in-place edit is acceptable here.)

Author response: dismissed with a verified rationale (confirmed the key's introduction date before accepting the explanation).

Important — Should address before merge

• share-layout.component.tsx: Removing tw:font-medium from the "Bible texts"/"Project commentaries"/"Text collection resources" labels (to match the plain "Model text"/"Default tab" labels) moved away from the codebase's established convention for header labels in row-style cards, which consistently use font-medium (e.g. settings-list.component.tsx, comment-item.component.tsx, conflict-note-card.component.tsx). (fixed during review: reversed direction — added tw:font-medium to the "Model text" and "Default tab" labels instead, so all 5 labels are now bold and consistent with the rest of the app's convention.)
• assets/localization/en.json / share-layout.stories.tsx: The renamed label used lowercase "bible" ("Project bible texts") while "Bible" is capitalized as a proper noun elsewhere in the very same dialog/file (%shareLayoutDialog_activeTab_scriptureResource% = "Bible texts", shown in this dialog's own "Default tab" dropdown). (fixed during review: capitalized to "Project Bible texts"; per author's follow-up request, also capitalized "Project commentaries" → "Project Commentaries" for consistency.)

Author response: both findings were addressed immediately with code changes during the interview.

Minor — Consider

• share-layout.component.tsx:362-407: The merged resource card manually rebuilds a bordered/rounded card (rounded-xl border bg-muted/30) rather than using the shared Card/CardHeader/CardAction components. This wrapper predates this change (already existed before the merge) — a standing opportunity, not a regression introduced here.
• platform-scripture-editor.web-view.tsx:1873: The new !isPowerMode guard around ShareLayoutButton has no test coverage, but neither do the existing sibling conditionals it's patterned after (bcvControls, scrollGroupSelector) — not a new gap.
• assets/localization/es.json: The Spanish translation "Textos bíblicos" was stale relative to the updated English value. (fixed during review: retranslated to "Textos bíblicos del proyecto", following the "del proyecto" pattern already used for the sibling commentary label.)

Author response: requested a fix for the stale Spanish translation (done); explicitly chose to leave the other two open for now.

Template Propagation

Shared Regions Modified

None. — no #region shared with <url> markers in any changed file.

Extension Config Changes

• [n/a] extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx — a web-view source file, not a config/build file; no propagation needed.

Positive Observations

• The power-mode-hide change reuses the file's existing isPowerMode variable and conditional-render pattern (already used for bcvControls/scrollGroupSelector) rather than introducing new state or a duplicate check, with a clear code comment explaining the simple-mode-only rationale.
• The card merge, divider restructuring, and label changes in share-layout.component.tsx were verified line-for-line against the stated purpose — no stray or unrelated styling changes.
• The SelectTrigger tw:bg-background fix correctly root-causes the "grayed out" look (shadcn's SelectTrigger defaults to tw:bg-transparent, which was blending into the card's bg-muted/30) without touching the shared shadcn component — no CUSTOM comment needed since the shared component itself wasn't modified.
• share-layout.stories.tsx was kept in sync with every label-value change (the rename plus both capitalization fixes), so the Storybook mock never went stale.
• The added tw:text-sm on resource entry text matches the effective text size ResourcePickerDialog already uses elsewhere, keeping typography consistent across the two resource-selection surfaces.
• The change set stayed tightly scoped to exactly the files needed for the stated purpose, with no incidental refactoring.

Interview Notes

• Stated purpose: Hide the Share Layout toolbar button in power mode, and polish the Share Layout dialog's UI (merge cards, remove dividers, match label fonts, rename/capitalize a label, fix a dropdown's background).
• The author made several explicit, well-reasoned design calls during the interview:
  • Confirmed (after I verified via git log) that the in-place localization string edit was acceptable because the key is only 2 days old with no shipped downstream consumers.
  • Chose to make all 5 dialog labels bold (font-medium) rather than removing weight from the three resource-card labels, aligning with the codebase's established header-label convention.
  • Requested "Project Bible texts" and "Project Commentaries" capitalization for consistency with "Bible" as a proper noun elsewhere in the app.
  • Asked for the stale Spanish translation to be fixed, but explicitly deprioritized the Card component adoption and missing test coverage minor findings.
• No unresolved items — the one Critical finding was dismissed with a verified rationale, and both Important findings were fixed in-review. The author demonstrated clear understanding of every change discussed; no signals of deferred/unclear understanding.

In-Review Quality Check

• npm run typecheck — PASS, no errors.
• npm run lint — PASS, 0 errors (2 pre-existing unrelated warnings elsewhere).
• Prettier (scoped to the review diff) — PASS, all 5 changed files already correctly formatted.
• npm test — PASS for everything relevant: all share-layout.* suites green (share-layout.component.test.tsx 10/10, share-layout.dialog.test.tsx 8/8, share-layout.utils.test.ts 14/14, share-layout-button.component.test.tsx 5/5).
• Unrelated, not fixed: 6 Storybook/Vitest browser-mode suites in the platform-bible-react workspace fail with a stale dependency-cache error (does not provide an export named 'getNextChapterRef' etc.) — verified those exports genuinely exist in current source; this is a stale node_modules/.cache/storybook artifact unrelated to any file touched by this branch, left alone per scope.

Suggested Review Focus

• Confirm the team is comfortable with the in-place localization string edit given the "2 days old, no consumers yet" rationale, versus requiring the metadata.fallbackKey pattern even for very recent strings.
• Optional: consider adopting Card/CardHeader/CardAction for the share-layout dialog's resource cards next time this file is touched (pre-existing pattern, not introduced by this branch).
• Optional: no test coverage for the power-mode toolbar-button visibility change — consistent with sibling conditionals, but worth deciding if a coverage bar should be raised here.

AI-assisted — session

---

This change is 

[tombogle]

@tombogle reviewed 5 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on captaincrazybro).

---

-- commits line 29 at r1:
When you reword this for the squash-merge, you might want to smooth it out. As stated, it implies that capitalizing "Project Commentaries" is necessary to be consistent with treating "Bible" as a proper nound.

---

assets/localization/en.json line 406 at r1 (raw file):

  "%shareLayoutDialog_modelText_label%": "Model text",
  "%shareLayoutDialog_modelText_none%": "None selected",
  "%shareLayoutDialog_scriptureResources_label%": "Project Bible texts",

So, obviously "Bible" needed to be capitalized because it is a proper name. "Commentaries" is not. Presumably, the consistency we're after is with

      "%webView_resourcePanel_bibleTexts_title%": "Bible texts",
      ...
      "%webView_resourcePanel_commentaries_title%": "Commentaries",

If we really intend for these to stay connected, maybe we should use params so that (in most localizations), they will be:

'%shareLayoutDialog_scriptureResources_label%': 'Project {bibleTextsPanelTitle}',
'%shareLayoutDialog_commentaryResources_label%': 'Project {commentaryPanelTitle}',

But since those titles come from the extension and these don't, they would actually need to be turned around. As it is, we have an implicit link between the core renderer components and the extensions. This same argument applies (even more so) to the '%shareLayoutDialog_activeTab_' IDs above, which are verbatim copies of the tab titles in the extension. If these are allowed to get out of sync, I suspect that it would be pretty bad/confusing for the user. I'm not sure what the best way is to ensure a single source of truth, but something to talk to Claude about and/or discuss with people who have a good grasp on the architecture.

---

assets/localization/es.json line 524 at r1 (raw file):

  "%shareLayoutDialog_modelText_label%": "Texto modelo",
  "%shareLayoutDialog_modelText_none%": "Ninguno seleccionado",
  "%shareLayoutDialog_scriptureResources_label%": "Textos bíblicos del proyecto",

In English, "Project Bible texts" is already somewhat ambiguous. A native speaker could reasonably interpret it as the Bible text belonging to the project rather than Bible texts associated with the project as reference resources.

In Spanish, "Textos bíblicos del proyecto" strengthens that interpretation. The construction "del proyecto" naturally reads as "the project's Bible texts"—i.e., the texts being produced by the project.

Consider these alternatives:
Textos bíblicos de referencia del proyecto
Recursos bíblicos del proyecto
Textos bíblicos de apoyo para el proyecto

I actually prefer the middle (shortest) one, but UX might have to weigh in on whether we're okay using "recursos" in Spanish when we seem to be trying to avoid the term "resources" in English.

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1871 at r1 (raw file):

              className="tw:h-8"
            />
            {/* Share Layout is a simple-mode-only action; power mode's scroll groups replace the

Share Layout is available only in Simple mode; Power mode's more complex and user-customizable layout with multiple scroll groups...

The multiple-part compound adjective and lowercase "simple"; caused me to mis-parse it and arrive at the wrong understanding.

[tombogle]

assets/localization/en.json line 406 at r1 (raw file):

Previously, tombogle (Tom Bogle) wrote…

So, obviously "Bible" needed to be capitalized because it is a proper name. "Commentaries" is not. Presumably, the consistency we're after is with

      "%webView_resourcePanel_bibleTexts_title%": "Bible texts",
      ...
      "%webView_resourcePanel_commentaries_title%": "Commentaries",

If we really intend for these to stay connected, maybe we should use params so that (in most localizations), they will be:

'%shareLayoutDialog_scriptureResources_label%': 'Project {bibleTextsPanelTitle}',
'%shareLayoutDialog_commentaryResources_label%': 'Project {commentaryPanelTitle}',

But since those titles come from the extension and these don't, they would actually need to be turned around. As it is, we have an implicit link between the core renderer components and the extensions. This same argument applies (even more so) to the '%shareLayoutDialog_activeTab_' IDs above, which are verbatim copies of the tab titles in the extension. If these are allowed to get out of sync, I suspect that it would be pretty bad/confusing for the user. I'm not sure what the best way is to ensure a single source of truth, but something to talk to Claude about and/or discuss with people who have a good grasp on the architecture.

Called out in https://paratextstudio.atlassian.net/browse/PT-4216

[tombogle]

@tombogle reviewed 4 files and all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on captaincrazybro).

[tombogle]

@tombogle made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on captaincrazybro).