refactor(msgbox): add customizable DialogControl + Dialog API#3168
refactor(msgbox): add customizable DialogControl + Dialog API#3168tangge233 wants to merge 14 commits into
Conversation
Adds DialogControl (chrome with ContentPresenter), Dialog static API, DialogContext/DialogTheme/DialogButton types. MyMsgBoxTick now routes FrameworkElement content to DialogControl. MsgBoxWrapper delegates to Dialog, old types marked [Obsolete].
pcl-ce-automation
Bot
added
🛠️ 等待审查
Reviewer's Guide引入了新的可复用 新的
|
| Change | Details | Files |
|---|---|---|
将 MyMsgBoxTick 路由为在提供 FrameworkElement 类型的 Content 时渲染新的 DialogControl，否则渲染传统的 MyMsg* 窗口，并新增 Dialog_OnShow，将 DialogContext 转换为 MyMsgBoxConverter 条目。 |
|
Plain Craft Launcher 2/Modules/ModMain.cs |
弃用 MsgBoxWrapper API，改用新的 Dialog API，并在内部将 ShowWithCustomButtons 转发到 Dialog.Show。 |
|
PCL.Core/UI/MsgBoxWrapper.cs |
在 FormMain 中注册新的 Dialog.OnShow 处理程序，将 Dialog API 挂接到主窗口生命周期。 |
|
Plain Craft Launcher 2/FormMain.xaml.cs |
添加可复用的 WPF 控件 DialogControl，用于承载任意内容，管理按钮、动画以及模态关闭语义。 |
|
Plain Craft Launcher 2/Controls/Dialog/DialogControl.csPlain Craft Launcher 2/Controls/Dialog/DialogControl.xaml |
引入新的 Dialog API 表面（Dialog、DialogButton、DialogTheme、DialogContext）用于应用范围内的对话框使用。 |
|
PCL.Core/UI/Dialog.cs |
Tips and commands
Interacting with Sourcery
- 触发新的代码审查： 在 Pull Request 中评论
@sourcery-ai review。 - 继续讨论： 直接回复 Sourcery 的审查评论。
- 从审查评论生成 GitHub Issue： 通过回复审查评论，请求 Sourcery 从该评论创建一个 Issue。你也可以直接在审查评论下回复
@sourcery-ai issue来从该评论创建 Issue。 - 生成 Pull Request 标题： 在 Pull Request 标题中任意位置写上
@sourcery-ai，即可随时生成标题。你也可以在 Pull Request 中评论@sourcery-ai title来（重新）生成标题。 - 生成 Pull Request 摘要： 在 Pull Request 正文中任意位置写上
@sourcery-ai summary，即可在该位置生成 PR 摘要。你也可以在 Pull Request 中评论@sourcery-ai summary来在任意时刻（重新）生成摘要。 - 生成审查者指南： 在 Pull Request 中评论
@sourcery-ai guide，即可在任意时刻（重新）生成审查者指南。 - 解决所有 Sourcery 评论： 在 Pull Request 中评论
@sourcery-ai resolve，即可一次性解决所有 Sourcery 评论。如果你已经处理完所有评论且不想再看到它们，这会非常有用。 - 撤销所有 Sourcery 审查： 在 Pull Request 中评论
@sourcery-ai dismiss，即可撤销所有现有的 Sourcery 审查。尤其适用于你想重新开始一次全新的审查——别忘了再评论一次@sourcery-ai review来触发新的审查！
Customizing Your Experience
打开你的 dashboard 可以：
- 启用或禁用审查功能，例如 Sourcery 自动生成的 Pull Request 摘要、审查者指南等。
- 更改审查语言。
- 添加、删除或编辑自定义审查说明。
- 调整其他审查设置。
Getting Help
Original review guide in English
Reviewer's Guide
Introduces a new reusable DialogControl and Dialog static API, routes new dialog requests through Dialog_OnShow into the existing MyMsgBox infrastructure, and deprecates the old MsgBoxWrapper-based types in favor of the new Dialog abstractions while keeping existing behavior compatible.
Sequence diagram for the new Dialog.Show flow
sequenceDiagram
actor Caller
participant Dialog
participant FormMain
participant ModMain
participant DialogControl
Caller->>Dialog: Show(caption, title, theme, block, buttons)
activate Dialog
Dialog->>Dialog: BuildButtons(buttons, theme)
Dialog->>Dialog: Show(DialogContext)
Dialog-->>FormMain: OnShow(DialogContext)
deactivate Dialog
activate FormMain
FormMain-->>ModMain: Dialog_OnShow(DialogContext)
deactivate FormMain
activate ModMain
ModMain->>ModMain: Create MyMsgBoxConverter
ModMain->>ModMain: WaitingMyMsgBox.Add(converter)
ModMain->>ModMain: MyMsgBoxTick()
ModMain->>DialogControl: new DialogControl()
ModMain->>DialogControl: AddButton(button1)
ModMain->>DialogControl: AddButton(button2)
ModMain->>DialogControl: AddButton(button3)
ModMain->>DialogControl: subscribe OnClosed
deactivate ModMain
activate DialogControl
DialogControl-->>ModMain: OnClosed(result)
deactivate DialogControl
activate ModMain
ModMain->>Dialog: set DialogContext.Result
deactivate ModMain
File-Level Changes
| Change | Details | Files |
|---|---|---|
| Route MyMsgBoxTick to render either legacy MyMsg* windows or the new DialogControl when a FrameworkElement Content is provided, and add Dialog_OnShow to translate DialogContext into MyMsgBoxConverter entries. |
|
Plain Craft Launcher 2/Modules/ModMain.cs |
| Deprecate MsgBoxWrapper API in favor of the new Dialog API and internally forward ShowWithCustomButtons to Dialog.Show. |
|
PCL.Core/UI/MsgBoxWrapper.cs |
| Register the new Dialog.OnShow handler in FormMain to hook the Dialog API into the main window lifecycle. |
|
Plain Craft Launcher 2/FormMain.xaml.cs |
| Add a reusable DialogControl WPF control that hosts arbitrary content, manages buttons, animations, and modal closing semantics. |
|
Plain Craft Launcher 2/Controls/Dialog/DialogControl.csPlain Craft Launcher 2/Controls/Dialog/DialogControl.xaml |
| Introduce a new Dialog API surface (Dialog, DialogButton, DialogTheme, DialogContext) for app-wide dialog usage. |
|
PCL.Core/UI/Dialog.cs |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些高层次的反馈：
- 在
MsgBoxWrapper.ShowWithCustomButtons中通过强制转换(DialogTheme)(int)theme将MsgBoxTheme转为DialogTheme的方式比较脆弱，如果任一枚举的顺序发生变化就会出错；建议改为显式映射（例如使用 switch）。 - 在构造
MyMsgBoxConverter/DialogControl时，从未使用DialogButton.IsPrimary属性（只有第一个按钮被视为主按钮）；建议在创建按钮时尊重IsPrimary，或者移除该属性以避免混淆。 - 在
DialogControl.AddButton中，当提供了onClick处理程序时，对话框不会被关闭，也不会设置结果，这与没有处理程序的按钮行为不同；建议要么在文档中明确这一约定，要么提供一个在onClick后自动关闭的选项。
面向 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The conversion from `MsgBoxTheme` to `DialogTheme` via direct cast `(DialogTheme)(int)theme` in `MsgBoxWrapper.ShowWithCustomButtons` is brittle and will break if either enum changes order; consider an explicit mapping (e.g., switch) instead.
- The `DialogButton.IsPrimary` property is never used when constructing `MyMsgBoxConverter`/`DialogControl` (only the first button is treated as primary); either honor `IsPrimary` when creating buttons or remove the property to avoid confusion.
- In `DialogControl.AddButton`, when an `onClick` handler is provided the dialog is not closed and no result is set, which changes behavior compared to buttons without handlers; consider either documenting this contract or providing an option to auto-close after `onClick`.
## Individual Comments
### Comment 1
<location path="PCL.Core/UI/MsgBoxWrapper.cs" line_range="45-54" />
<code_context>
- var result = 0;
- if (buttonCollection.Count == 0) buttonCollection = [new MsgBoxButtonInfo(Lang.Text("Common.Action.Confirm"))];
- OnShow?.Invoke(message, caption, buttonCollection, theme, block, ref result);
+ var buttons = new Collection<DialogButton>();
+ foreach (var btn in buttonCollection)
+ {
+ buttons.Add(new DialogButton(btn.Context, btn.OnClick));
+ }
+ var result = Dialog.Show(new DialogContext
+ {
+ Caption = message,
+ Title = caption,
+ Theme = (DialogTheme)(int)theme,
+ Block = block,
+ Content = null,
+ Buttons = buttons,
+ });
return result;
</code_context>
<issue_to_address>
**issue (bug_risk):** MsgBoxButtonInfo.Value is dropped, potentially breaking callers that depend on custom result codes.
Previously, `ShowWithCustomButtons` propagated each `MsgBoxButtonInfo.Value` into the `ref int result`, but the new implementation only carries `Context`/text and `OnClick` into `DialogButton` / `DialogContext.Result`. Callers that depended on specific `Value` codes will now see only positional indices (or 0 if never set). If this API is still expected to be backward-compatible, you’ll need to either pass `Value` through `DialogContext` and map it back on selection, or clearly document that `Value` is ignored and results are purely index-based.
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The conversion from
MsgBoxThemetoDialogThemevia direct cast(DialogTheme)(int)themeinMsgBoxWrapper.ShowWithCustomButtonsis brittle and will break if either enum changes order; consider an explicit mapping (e.g., switch) instead. - The
DialogButton.IsPrimaryproperty is never used when constructingMyMsgBoxConverter/DialogControl(only the first button is treated as primary); either honorIsPrimarywhen creating buttons or remove the property to avoid confusion. - In
DialogControl.AddButton, when anonClickhandler is provided the dialog is not closed and no result is set, which changes behavior compared to buttons without handlers; consider either documenting this contract or providing an option to auto-close afteronClick.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The conversion from `MsgBoxTheme` to `DialogTheme` via direct cast `(DialogTheme)(int)theme` in `MsgBoxWrapper.ShowWithCustomButtons` is brittle and will break if either enum changes order; consider an explicit mapping (e.g., switch) instead.
- The `DialogButton.IsPrimary` property is never used when constructing `MyMsgBoxConverter`/`DialogControl` (only the first button is treated as primary); either honor `IsPrimary` when creating buttons or remove the property to avoid confusion.
- In `DialogControl.AddButton`, when an `onClick` handler is provided the dialog is not closed and no result is set, which changes behavior compared to buttons without handlers; consider either documenting this contract or providing an option to auto-close after `onClick`.
## Individual Comments
### Comment 1
<location path="PCL.Core/UI/MsgBoxWrapper.cs" line_range="45-54" />
<code_context>
- var result = 0;
- if (buttonCollection.Count == 0) buttonCollection = [new MsgBoxButtonInfo(Lang.Text("Common.Action.Confirm"))];
- OnShow?.Invoke(message, caption, buttonCollection, theme, block, ref result);
+ var buttons = new Collection<DialogButton>();
+ foreach (var btn in buttonCollection)
+ {
+ buttons.Add(new DialogButton(btn.Context, btn.OnClick));
+ }
+ var result = Dialog.Show(new DialogContext
+ {
+ Caption = message,
+ Title = caption,
+ Theme = (DialogTheme)(int)theme,
+ Block = block,
+ Content = null,
+ Buttons = buttons,
+ });
return result;
</code_context>
<issue_to_address>
**issue (bug_risk):** MsgBoxButtonInfo.Value is dropped, potentially breaking callers that depend on custom result codes.
Previously, `ShowWithCustomButtons` propagated each `MsgBoxButtonInfo.Value` into the `ref int result`, but the new implementation only carries `Context`/text and `OnClick` into `DialogButton` / `DialogContext.Result`. Callers that depended on specific `Value` codes will now see only positional indices (or 0 if never set). If this API is still expected to be backward-compatible, you’ll need to either pass `Value` through `DialogContext` and map it back on selection, or clearly document that `Value` is ignored and results are purely index-based.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| var buttons = new Collection<DialogButton>(); | ||
| foreach (var btn in buttonCollection) | ||
| { | ||
| buttons.Add(new DialogButton(btn.Context, btn.OnClick)); | ||
| } | ||
| var result = Dialog.Show(new DialogContext | ||
| { | ||
| Caption = message, | ||
| Title = caption, | ||
| Theme = (DialogTheme)(int)theme, |
There was a problem hiding this comment.
issue (bug_risk): MsgBoxButtonInfo.Value 被丢弃了，可能会影响依赖自定义结果代码的调用方。
之前，ShowWithCustomButtons 会把每个 MsgBoxButtonInfo.Value 传递给 ref int result，但新的实现只把 Context/文本和 OnClick 带入 DialogButton / DialogContext.Result。依赖特定 Value 码的调用方现在只能看到位置索引（或者在从未设置时为 0）。如果这个 API 仍然需要保持向后兼容，就需要要么通过 DialogContext 传递 Value 并在选择时映射回去，要么在文档中明确说明会忽略 Value，且结果仅基于索引。
Original comment in English
issue (bug_risk): MsgBoxButtonInfo.Value is dropped, potentially breaking callers that depend on custom result codes.
Previously, ShowWithCustomButtons propagated each MsgBoxButtonInfo.Value into the ref int result, but the new implementation only carries Context/text and OnClick into DialogButton / DialogContext.Result. Callers that depended on specific Value codes will now see only positional indices (or 0 if never set). If this API is still expected to be backward-compatible, you’ll need to either pass Value through DialogContext and map it back on selection, or clearly document that Value is ignored and results are purely index-based.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bef259d9aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| content = new TextBlock | ||
| { | ||
| Text = context.Caption, | ||
| TextWrapping = TextWrapping.Wrap, | ||
| FontSize = 15, |
There was a problem hiding this comment.
When Dialog.Show/MsgBoxWrapper.Show is called from a worker thread with only a caption, this allocates a WPF TextBlock on that worker and stores it in converter.Content. MyMsgBoxTick later attaches that FrameworkElement to DialogControl on the UI dispatcher, which WPF rejects because the element belongs to a different Dispatcher; affected paths include non-UI logging/config/dependency prompts, so the queued dialog can fail to render and blocking callers can hang. Queue the caption as data or create the element inside the UI tick instead.
Useful? React with 👍 / 👎.
| dialog.AddButton(converter.Button2, converter.Button2Action); | ||
| if (!string.IsNullOrEmpty(converter.Button3)) | ||
| dialog.AddButton(converter.Button3, converter.Button3Action); | ||
| converter.WaitFrame = dialog.WaitFrame; |
There was a problem hiding this comment.
For a blocking Dialog.Show from a non-UI thread, Dialog_OnShow starts waiting on the original converter.WaitFrame immediately after enqueuing the converter. When the UI tick later enters this branch, replacing that frame means DialogControl.Close() stops only the new frame, leaving the caller's original Dispatcher.PushFrame running forever; this can be hit by blocking MsgBoxWrapper.Show paths such as the missing-dependency prompt. Reuse the converter's existing frame or marshal the whole blocking show onto the UI thread before waiting.
Useful? React with 👍 / 👎.
BtnDialogTest in 百宝箱 demonstrates rich content in DialogControl: text, border, progress bar, color swatches, styled layout. Visible only in DEBUG builds.
DialogButton.Id returns on close instead of index. Presets: DialogButton.Confirm(), .Cancel(), .Yes(), .No() Constants: DialogResult.Ok=1, Cancel=2, Yes=6, No=7 All text via Lang.Text for i18n.
Dialog.ShowAsync() returns Task<int> for awaitable non-blocking. DialogContext.OnClosed callback fires on dialog close. MyMsgBoxConverter.OnCloseCallback bridges non-blocking path. Sync Dialog.Show() still works via Dispatcher.PushFrame.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a81caa7cfb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Button1 = context.Buttons.Count > 0 ? context.Buttons[0].Text : GetDefaultConfirmText(), | ||
| Button2 = context.Buttons.Count > 1 ? context.Buttons[1].Text : "", | ||
| Button3 = context.Buttons.Count > 2 ? context.Buttons[2].Text : "", |
There was a problem hiding this comment.
When a caller uses the new Dialog.Show(..., params string[] buttons) or supplies a DialogContext.Buttons collection with more than three entries, the public API builds all of them but this bridge only copies the first three into MyMsgBoxConverter. The remaining choices are silently dropped before MyMsgBoxTick creates the DialogControl, so a blocking caller can never receive the missing button IDs. Iterate over context.Buttons when adding buttons to DialogControl, or reject extra buttons explicitly.
Useful? React with 👍 / 👎.
| foreach (var btn in buttonCollection) | ||
| { | ||
| buttons.Add(new DialogButton(btn.Context, btn.OnClick)); | ||
| } |
There was a problem hiding this comment.
When legacy callers use MsgBoxWrapper.ShowWithCustomButtons with MsgBoxButtonInfo.Value values, this conversion drops the value and creates each DialogButton with the default id of 0. Dialog_OnShow then falls back to positional IDs, so selecting a button returns 1/2/3 instead of the caller-specified value; pass btn.Value into the DialogButton id to keep the existing result contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afb6f7464e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| if (context.Buttons.Count == 0) | ||
| context.Buttons.Add(DialogButton.Confirm()); | ||
| context.Block = true; |
There was a problem hiding this comment.
When callers pass block: false (for example LogService still uses MsgBoxWrapper.Show(..., false) for non-modal log popups), this assignment overwrites the requested value before Dialog_OnShow sees the context, so the UI bridge enters the blocking Dispatcher.PushFrame path anyway. This regresses the old fire-and-forget message boxes by making logging/background paths wait for user dismissal instead of just queuing the dialog.
Useful? React with 👍 / 👎.
Dialog_OnShow now creates DialogControl directly via Dispatcher.Invoke. No more converter/queue for new Dialog API. Old MyMsgBox path untouched. DialogControl.Close background check accounts for stacked dialogs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bd383f90b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| buttons.Add(new DialogButton(btn.Context, btn.OnClick)); | ||
| } | ||
| var result = Dialog.Show(new DialogContext |
There was a problem hiding this comment.
When existing code subscribes to MsgBoxWrapper.OnShow, this wrapper path now bypasses that public event and calls Dialog.Show(...) directly, so legacy hosts/tests that still rely on the obsolete API never get a chance to render the dialog or set the ref result. Since the event remains exposed and FormMain still registers a handler for it, either bridge/invoke the legacy event here or make the old API fail explicitly instead of silently ignoring subscribers.
Useful? React with 👍 / 👎.
MyMsgBoxTick now creates DialogControl for Text/Input/Select/Markdown. DialogControl.Close(int result) added for custom button handlers. Login still uses MyMsgLogin (complex OAuth lifecycle). Old MyMsgText/Input/Select/Markdown files kept, no longer instantiated.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38a2c5752
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| var converter = WaitingMyMsgBox[0]; | ||
| var dialog = CreateDialogFromConverter(converter); | ||
| if (dialog is not null) | ||
| frmMain.PanMsg.Children.Add(dialog); |
There was a problem hiding this comment.
Because queued MyMsgBox converters are now added as DialogControl instances here, the existing FormMain_KeyDown shortcut path no longer reaches them: it switches only on MyMsgInput/MyMsgSelect/MyMsgText/MyMsgMarkdown/MyMsgLogin and then returns. This means Enter/Esc are swallowed and do nothing for standard, input, select, and markdown dialogs that used to activate their primary/cancel buttons; add DialogControl handling or expose equivalent click methods.
Useful? React with 👍 / 👎.
| if (_buttons.Count > 0) | ||
| _buttons[0].Focus(); |
There was a problem hiding this comment.
DialogControl always focuses the first button during load. Since MyMsgBoxInput prompts are now built with this container, the text box no longer receives the caret as the old input dialog did, so users typing immediately into prompts such as instance/profile names type nowhere until they click the field. Let input dialogs request focus for their MyTextBox instead of unconditionally focusing the first button.
Useful? React with 👍 / 👎.
ModMain.cs: removed ~650 lines of dialog code, added 'partial' keyword. ModMain.Dialog.cs: new file with all dialog logic (MyMsgBoxConverter, MyMsgBox*, MyMsgBoxTick, Dialog_OnShow, CreateDialogFromConverter, etc.). Zero call-site changes — all public API remains on ModMain.
pcl-ce-automation
Bot
added
size: XXL
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 072e6416f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| var btn = context.Buttons[i]; | ||
| var isPrimary = i == 0 || btn.IsPrimary; | ||
| var id = btn.Id > 0 ? btn.Id : i + 1; | ||
| dialog.AddButton(btn.Text, btn.OnClick, isPrimary, id); |
There was a problem hiding this comment.
In the Dialog API path, passing btn.OnClick directly into DialogControl.AddButton uses the legacy action-only behavior: a non-null callback is executed but the dialog is not closed or assigned a result. When a caller uses Dialog.Show or Dialog.ShowAsync with a DialogButton that has an OnClick callback, especially as the only primary button, the caller has no dialog instance to close from that callback, so the dialog stays open and blocking/async callers never receive a result. Wrap the callback here and close with the button id, or make keeping the dialog open an explicit option.
Useful? React with 👍 / 👎.
Dialog/DialogManager.cs — orchestrator init with FormMain, Show/ShowAsync, ShowText/ShowMarkdown/ShowInput/ShowSelect conveniences, converter logic. Dialog/DialogContext.cs, DialogButton.cs, DialogResult.cs, DialogTheme.cs — type files (moved from PCL.Core.UI). Dialog/LegacyDialog.cs — partial ModMain: MyMsgBoxConverter, WaitingMyMsgBox, thin wrappers delegating to DialogManager.Instance. PCL.Core/UI/MsgBoxWrapper.cs — reverted to event pattern, no Dialog dependency. Removed: PCL.Core/UI/Dialog.cs, Modules/ModMain.Dialog.cs (merged above). ModMain.cs timer calls DialogManager.Instance.Tick().
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7ad35d169
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| var buttons = new List<string>(); | ||
| if (!string.IsNullOrEmpty(button1)) buttons.Add(button1!); | ||
| if (!string.IsNullOrEmpty(button2)) buttons.Add(button2!); | ||
| if (!string.IsNullOrEmpty(button3)) buttons.Add(button3!); |
There was a problem hiding this comment.
When callers omit button1 but pass button2 (for example the settings reset confirmations that pass only button2: Common.Action.Cancel and then check for result == 1), this list no longer inserts the default confirm button and instead promotes the cancel text to the first/only button. That button gets result id 1, so clicking the visible Cancel can execute the existing confirm branch; default button1 before building the list.
Useful? React with 👍 / 👎.
| return mgr.ShowText(caption, title, | ||
| isWarn ? DialogTheme.Warning : DialogTheme.Info, | ||
| forceWait || buttons.Count > 1, | ||
| buttons.ToArray()); |
There was a problem hiding this comment.
When existing callers pass button1Action/button2Action/button3Action to ModMain.MyMsgBox (such as copy-IP, open-download-page, or update-log buttons), this path converts only the button labels and calls ShowText, so those action parameters are discarded before DialogControl is built. The affected buttons now just close/return an id instead of performing their side effect.
Useful? React with 👍 / 👎.
| return Show(new DialogContext | ||
| { | ||
| Caption = markdown, | ||
| Title = title ?? Lang.Text("Common.Dialog.Title"), | ||
| Theme = theme, | ||
| Block = block, | ||
| Buttons = _BuildButtons(buttons), | ||
| // Markdown content is created in ShowOnUi via CreateDialogFromContext |
There was a problem hiding this comment.
When MyMsgBoxMarkdown/ShowMarkdown is used for changelogs or feedback text, this context carries only Caption and no markdown content marker; ShowDialogOnUiThread later handles null content by creating a plain TextBlock. As a result markdown is displayed raw and links/formatting are lost, so this should populate Content with a MarkdownViewer or otherwise route through the markdown-specific dialog path.
Useful? React with 👍 / 👎.
…rols MyMsgLogin now takes (data, authUrl, onFinished) callback — no converter. ModLaunch OAuth flow uses ManualResetEvent + MyMsgLogin directly. ShowInput/ShowSelect create DialogControl directly via Dispatcher.Invoke. Removed: MyMsgBoxConverter, MyMsgBoxType, WaitingMyMsgBox queue. Removed: MyMsgText/Input/Select/Markdown XAML+CS files. DialogManager.Tick simplified to background visibility only. Keyboard handler (Enter/Esc) updated for DialogControl.
ShowOnUi, ShowInput, ShowSelect now guard against dispatcher shutdown by catching TaskCanceledException when marshaling to UI thread.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be51f26410
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Action? button1Action = null, Action? button2Action = null, Action? button3Action = null) | ||
| { | ||
| var mgr = DialogManager.Instance; | ||
| if (mgr is null) return 1; |
There was a problem hiding this comment.
When MyMsgBox is called before new DialogManager(this) runs, this early return silently reports success without queuing or displaying anything. FormMain still calls UpgradeSub(...) in its constructor before the manager is created, and that path invokes UpdateManager.ShowCEAnnounce() via MyMsgBox, so the required upgrade/community notice is skipped instead of being shown after the UI finishes loading.
Useful? React with 👍 / 👎.
| Theme = theme, | ||
| Block = block, | ||
| Buttons = _BuildButtons(buttons), | ||
| Content = new Markdig.Wpf.MarkdownViewer { Markdown = markdown }, |
There was a problem hiding this comment.
ShowMarkdown constructs a WPF MarkdownViewer on the caller's thread before ShowOnUi marshals to the main dispatcher. Calls such as FormMain.ShowUpdateLog() run MyMsgBoxMarkdown from a background thread, so the viewer is created with the wrong Dispatcher and later attached to DialogControl on the UI thread, which WPF rejects; create this control inside ShowDialogOnUiThread or pass markdown text as data.
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| _mainForm.Dispatcher.Invoke(showOnUi); | ||
| return result ?? ""; |
There was a problem hiding this comment.
Cancelling an input dialog now returns an empty string instead of the legacy null. Several callers distinguish cancellation with is not null checks, for example editing favorite notes or instance descriptions, so pressing Cancel can overwrite the existing value with ""; return null on the cancel path and keep the legacy contract.
Useful? React with 👍 / 👎.
| MyMsgText text => () => text.Btn1_Click(sender, null), | ||
| MyMsgMarkdown markdown => () => markdown.Btn1_Click(sender, null), | ||
| DialogControl dlg => dlg._buttons.Count > 0 | ||
| ? () => dlg._buttons[0].RaiseEvent(new RoutedEventArgs(Button.ClickEvent)) |
There was a problem hiding this comment.
The current handler still raises WPF's Button.ClickEvent, but DialogControl stores MyButton instances, which are Border-based controls exposing their own custom Click event; handlers registered in DialogControl.AddButton therefore are not invoked. In any DialogControl dialog, pressing Enter or Esc is consumed here but does not activate the intended button; invoke the MyButton click path or expose a method to close by button id instead.
Useful? React with 👍 / 👎.
pcl-ce-automation
Bot
added
🚧 正在处理
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dec5229c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| dialog.AddButton(btn.Text, btn.OnClick, isPrimary, id); | ||
| } | ||
|
|
||
| _mainForm.PanMsg.Children.Add(dialog); |
There was a problem hiding this comment.
When another dialog is already visible, this appends the new one directly to PanMsg instead of waiting for the current child to close as the old WaitingMyMsgBox tick did. WPF renders the later child on top, but FormMain_KeyDown still dispatches Enter/Esc to PanMsg.Children[0], so two background/dialog calls that overlap can make keyboard shortcuts close the hidden older dialog while the visible one remains open; restore a queue or make all dialog interaction target the topmost child.
Useful? React with 👍 / 👎.
… refactor DialogControl: ShowTitle property, isCancel in AddButton, auto-collapse button row. DialogButton: IsCancel, Cancel/No presets set it. DialogContext: ShowTitle. DialogManager: Show(UIElement) returns handle, Show<T>() with GetResultAsync(). Dialog.cs: Dialog<T> generic wrapper. Legacy: last multi-button auto-marked cancel. FormMain_KeyDown: Enter→first non-cancel, Esc→first cancel, no fallback.
…howOAuth MyMsgLogin.xaml/.cs → DialogMsOAuthLogin.xaml/.cs DialogManager.ShowOAuth(data, authUrl) encapsulates creation + blocking. ModLaunch.cs simplified to single call: DialogManager.Instance.ShowOAuth(...) FormMain keyboard handler updated for new class name.
… Tag hack AddButton now takes DialogButton (not raw params). ButtonBuilder constructs MyButton. _buttons stores (DialogButton, MyButton) tuple preserving metadata. Cancel detection uses Data.IsCancel instead of Tag string hack. All callers updated.
1 participant
Adds DialogControl (chrome with ContentPresenter), Dialog static API, DialogContext/DialogTheme/DialogButton types. MyMsgBoxTick now routes FrameworkElement content to DialogControl. MsgBoxWrapper delegates to Dialog, old types marked [Obsolete].
Summary by Sourcery
引入一个可自定义的对话框系统，并将现有的消息框基础设施通过该系统进行统一处理。
New Features:
DialogControl，支持任意内容以及可配置按钮。Dialog，包含DialogContext、DialogButton和DialogTheme，用于标准化对话框的创建和展示。Enhancements:
FrameworkElement内容时，将MyMsgBoxTick路由到DialogControl，并保留现有消息类型作为回退方案。Dialog.OnShow事件，同时继续处理现有的消息和提示事件。MsgBoxWrapper.ShowWithCustomButtons集成到新的DialogAPI 中，以复用统一的对话框处理管线。Chores:
MsgBoxWrapper相关的类型标记为已过时，推荐使用基于新Dialog的 API。Original summary in English
Summary by Sourcery
Introduce a customizable dialog system and route existing message box infrastructure through it.
New Features:
Enhancements:
Chores: