Skip to content

Collection Settings in React: dialog shell and save pipeline (BL-16902) - #8388

Draft
JohnThomson wants to merge 2 commits into
masterfrom
BL-16902-settings-shell
Draft

JohnThomson wants to merge 2 commits into
masterfrom
BL-16902-settings-shell

Conversation

@JohnThomson

@JohnThomson JohnThomson commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Problem

BL-16271 is moving Bloom's collection Settings dialog off WinForms and onto React with Config-R, and nothing on master was ready for that work to start. The React side was an empty two-page skeleton that nothing launched, backed by a placeholder endpoint. Everything that actually saves collection settings lived inside the WinForms dialog's OK handler, reachable from anywhere else only through a static pointer to the currently-open WinForms dialog. None of the seven per-tab cards could begin against that.

What the PR does

  • A real React dialog shell. Seven Config-R pages in the new design's order, with localized labels and placeholder content, sized like Book Settings. A second Settings button on the collections toolbar opens it; the WinForms button is relabelled "Old Settings" until the cutover.
  • Opened from the browser, from any tab. showCollectionSettingsDialog(pageKey?) raises the dialog's launch event directly, with no round trip through C#, and App renders the dialog, so later callers on the Edit or Publish tab can open it too, on a named page.
  • Real save behaviour. OK saves everything in one POST and Cancel posts nothing. When a changed value needs a restart, the dialog shows the "Bloom will close and re-open" reminder and relabels OK as "Restart". A validation problem is shown in place instead of closing. Each open starts from freshly fetched values, OK is held while a save is in flight, and a save that fails outright is reported and leaves the user's edits intact to try again.
  • Team Collection administrators only. GET collection/settings applies the same rule as the old button: a member who is not an administrator gets just the reason, in a small dialog with Close, and no editing session is opened.
  • One place where pending settings live. A PendingCollectionSettings session replaces the static DialogBeingEdited pointer, so both dialogs and the React tab components that already exist (Subscription, Team Collection, bookshelf) read and write the same pending state during the overlap. A post that lands when no dialog is open (a control losing focus as one closes) is ignored rather than failing.
  • The save logic is now testable. CollectionSettingsUpdater.Validate/Apply holds what the WinForms OK handler used to do, with unit tests. The legacy dialog keeps working unchanged through it and the shared session.

The plan for the rest of BL-16271, with its settled decisions, is docs/collection-settings-react/PLAN.md.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16902

🤖 Generated with Claude Code

Devin review


This change is Reviewable

JohnThomson added a commit that referenced this pull request Sep 22, 2026
Disabling OK while the save is in flight left it disabled for good if the POST
itself failed, so the only way out was Cancel, which throws away the edits. The
dialog now re-enables OK when the save fails, and a test covers it.

Found by Devin on PR #8388.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx Outdated
Comment thread src/BloomExe/web/controllers/CollectionSettingsApi.cs
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread DistFiles/localization/en/Bloom.xlf
JohnThomson added a commit that referenced this pull request Sep 22, 2026
…902)

The error callback added a moment ago swallowed the failure: bloomApi only
reports an error when nothing handles it, so a save that failed outright went
by with no sign at all. The callback now re-throws after re-enabling OK, so the
failure still reaches Bloom's usual error reporting, and the test checks that.

Found by Devin on PR #8388.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
@JohnThomson

Copy link
Copy Markdown
Contributor Author

[Claude Opus (subagent of Claude Fable 5.1) from John Thomson's machine during preflight]

Consulted Devin on 2026-09-22, up to commit 4e6ba3e568329b1f61f02747f494cb027642411a.

Across three rounds it raised five bugs and two things to investigate. Two of the bugs were about the new React Settings dialog stranding the user after a failed save — one leaving OK disabled for good, one then hiding the failure entirely — and both are fixed (3f5bd0f, 4e6ba3e); it also confirmed the re-open-with-stale-values fix from dbed175. Its question about Cancel's request racing a quick re-open is answered on its own thread and resolved.

Three threads are deliberately left open, waiting on the developer: posting a null third language does not remove it, retrying a save whose response was lost would hit an ended session, and the four new page labels went into the top-priority XLF without the priority being confirmed first. Each has its own thread on this PR.

CI (pr-automation) is green for this commit. CodeRabbit's auto-review is off in .coderabbit.yml and there is no Greptile configuration, so neither posts here.

@JohnThomson

Copy link
Copy Markdown
Contributor Author

[Claude Opus (subagent of Claude Fable 5.1) from John Thomson's machine during preflight]

Consulted Devin again on 2026-09-22, up to commit 6983f37172bbaab9797559e9fb38260c7dbb3513.

Nothing new. The review of this commit repeats the set it already raised, and now also considers the third-language question answered by the note added to the TypeScript contract. Of the threads that were waiting on the developer, two are decided and resolved above — the no-session posts are ignored again, and a null third language is defined to mean "leave it alone", with an empty tag being how you remove one. The XLF priority question is still open for John.

CI (pr-automation) is green for this commit.

Comment thread src/BloomExe/web/controllers/SubscriptionSettingsEditorApi.cs Outdated
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread src/BloomBrowserUI/collection/CollectionSettingsDialog.tsx
Comment thread src/BloomExe/web/controllers/SubscriptionSettingsEditorApi.cs Outdated
@andrew-polk

Copy link
Copy Markdown
Contributor

[Claude Opus 5.5 during preflight] Consulted Devin on 2026-09-25 22:31 -07:00 up to commit ee2e072. Across this run it raised four new bugs (subscription revert saving the undone code, and its lingering restart; Cancel during a save; a late settings reply after reopening) and two Investigate flags; all are fixed or answered on their own threads and resolved. Its final pass on the head commit found nothing new.

andrew-polk and others added 2 commits September 26, 2026 07:34
…er traps

Found while hand-testing BL-16902: clicking WinForms tabs from DPI-unaware PowerShell, pointing Bloom at a scratch collection, the collection-rename relaunch, and the workspace page's changing URL.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Seven placeholder Config-R pages opened from a second Settings button, from the browser on any tab. OK saves once through CollectionSettingsUpdater, shared with the WinForms dialog via a PendingCollectionSettings session; restart reminder, validation errors, retry after a failed save, and an administrators-only message in Team Collections. Plan for the rest of BL-16271 in docs/collection-settings-react/PLAN.md.

Co-authored-by: John Thomson <john_thomson@sil.org>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@andrew-polk
andrew-polk force-pushed the BL-16902-settings-shell branch from ee2e072 to 075c110 Compare September 26, 2026 14:34
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