feat(mymsgbox): 实现部分控件的 MVVM 设计#3201
Conversation
Reviewer's Guide通过添加用于 MyMsg 的数据模型、命令和视图模型,并将其部分接入现有基础设施,同时增加用于基于命令的按钮处理和带索引枚举的辅助工具,引入一个 MVVM 风格的消息框系统。 MVVM MyMsg 消息框创建与按钮点击时序图sequenceDiagram
actor User
participant ModMain
participant WaitMyMsgData
participant MyMsgText
participant MyMsgTextViewModel
participant MyMsgTextData
participant MyMsgBoxCommand
participant ButtonContext
User->>ModMain: MyMsgBox(MyMsgTextData data)
ModMain->>WaitMyMsgData: add(data)
ModMain->>ModMain: MyMsgBoxShow()
ModMain->>MyMsgText: new MyMsgText(WaitMyMsgData[0])
MyMsgText->>MyMsgTextData: subscribe Exited += Close
MyMsgText->>MyMsgTextViewModel: new MyMsgTextViewModel(data)
MyMsgTextViewModel->>MyMsgTextData: read Title, Message, Command
Note over MyMsgTextData,MyMsgBoxCommand: MyMsgTextData(message, buttons) creates MyMsgBoxCommand
MyMsgTextData->>MyMsgBoxCommand: new MyMsgBoxCommand(buttons)
User->>MyMsgBoxCommand: Execute(buttonId)
MyMsgBoxCommand->>MyMsgBoxCommand: Clicked(buttonId)
MyMsgBoxCommand->>ButtonContext: Operation.Invoke(buttonId)
alt ExitWhenClick is true
MyMsgBoxCommand->>MyMsgBoxCommand: Exited()
MyMsgBoxCommand->>MyMsgTextData: set Result
MyMsgTextData->>MyMsgTextData: Exited()
MyMsgTextData->>MyMsgText: Close()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideIntroduces an MVVM-style message box system by adding data models, commands, and view models for MyMsg, partially wiring them into existing infrastructure and adding helper utilities for command-based button handling and indexed enumeration. Sequence diagram for MVVM MyMsg box creation and button clicksequenceDiagram
actor User
participant ModMain
participant WaitMyMsgData
participant MyMsgText
participant MyMsgTextViewModel
participant MyMsgTextData
participant MyMsgBoxCommand
participant ButtonContext
User->>ModMain: MyMsgBox(MyMsgTextData data)
ModMain->>WaitMyMsgData: add(data)
ModMain->>ModMain: MyMsgBoxShow()
ModMain->>MyMsgText: new MyMsgText(WaitMyMsgData[0])
MyMsgText->>MyMsgTextData: subscribe Exited += Close
MyMsgText->>MyMsgTextViewModel: new MyMsgTextViewModel(data)
MyMsgTextViewModel->>MyMsgTextData: read Title, Message, Command
Note over MyMsgTextData,MyMsgBoxCommand: MyMsgTextData(message, buttons) creates MyMsgBoxCommand
MyMsgTextData->>MyMsgBoxCommand: new MyMsgBoxCommand(buttons)
User->>MyMsgBoxCommand: Execute(buttonId)
MyMsgBoxCommand->>MyMsgBoxCommand: Clicked(buttonId)
MyMsgBoxCommand->>ButtonContext: Operation.Invoke(buttonId)
alt ExitWhenClick is true
MyMsgBoxCommand->>MyMsgBoxCommand: Exited()
MyMsgBoxCommand->>MyMsgTextData: set Result
MyMsgTextData->>MyMsgTextData: Exited()
MyMsgTextData->>MyMsgText: Close()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
ModMain中新的MyMsgBox(MyMsgTextData data)重载目前是空实现;请实现预期行为(将基于 MVVM 的消息框入队/显示),或者显式抛出NotImplementedException,以避免静默无效果的调用。MyMsgTextViewModel的属性在 setter 中使用了field,但没有对应的后备字段,而且使用了无效的 C# 语法(get; set { ... }),因此该类按当前写法将无法编译;请重构每个属性,使用显式的后备字段并正确调用SetProperty。MyMsgBoxShow同时混用了WaitingMyMsgBox和WaitMyMsgData(检查的是WaitingMyMsgBox.Count,却从WaitMyMsgData中出队),这会导致队列不同步,并可能引发索引错误或漏掉消息;请将条件判断和出队来源统一到同一个一致的集合上。
给 AI 代理的提示
请解决本次代码审查中的各条评论:
## 总体评论
- `ModMain` 中新的 `MyMsgBox(MyMsgTextData data)` 重载目前是空实现;请实现预期行为(将基于 MVVM 的消息框入队/显示),或者显式抛出 `NotImplementedException`,以避免静默无效果的调用。
- `MyMsgTextViewModel` 的属性在 setter 中使用了 `field`,但没有对应的后备字段,而且使用了无效的 C# 语法(`get; set { ... }`),因此该类按当前写法将无法编译;请重构每个属性,使用显式的后备字段并正确调用 `SetProperty`。
- `MyMsgBoxShow` 同时混用了 `WaitingMyMsgBox` 和 `WaitMyMsgData`(检查的是 `WaitingMyMsgBox.Count`,却从 `WaitMyMsgData` 中出队),这会导致队列不同步,并可能引发索引错误或漏掉消息;请将条件判断和出队来源统一到同一个一致的集合上。帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The new
MyMsgBox(MyMsgTextData data)overload inModMainis currently empty; either implement the intended behavior (enqueue/show the MVVM-based message box) or explicitly throwNotImplementedExceptionto avoid silent no-ops. - The
MyMsgTextViewModelproperties usefieldin the setter without backing fields and with invalid C# syntax (get; set { ... }), so this class as written will not compile; refactor each property to use explicit backing fields and callSetPropertycorrectly. MyMsgBoxShowmixesWaitingMyMsgBoxandWaitMyMsgData(checkingWaitingMyMsgBox.Countbut dequeuing fromWaitMyMsgData), which can desynchronize the queues and cause index errors or skipped messages; align the conditions and dequeue source to a single consistent collection.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `MyMsgBox(MyMsgTextData data)` overload in `ModMain` is currently empty; either implement the intended behavior (enqueue/show the MVVM-based message box) or explicitly throw `NotImplementedException` to avoid silent no-ops.
- The `MyMsgTextViewModel` properties use `field` in the setter without backing fields and with invalid C# syntax (`get; set { ... }`), so this class as written will not compile; refactor each property to use explicit backing fields and call `SetProperty` correctly.
- `MyMsgBoxShow` mixes `WaitingMyMsgBox` and `WaitMyMsgData` (checking `WaitingMyMsgBox.Count` but dequeuing from `WaitMyMsgData`), which can desynchronize the queues and cause index errors or skipped messages; align the conditions and dequeue source to a single consistent collection.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fdb4f5fcc
ℹ️ 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".
| public static int MyMsgBox(MyMsgTextData data) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
Implement the MyMsgTextData overload before merging
This new overload is declared to return int but falls off the end of the method. Since ModMain.cs is compiled into the WPF app, any build including this commit fails with CS0161 before the launcher can run; either implement the queue/wait logic and return the selected result or remove the overload until it is ready.
Useful? React with 👍 / 👎.
| Button1Text = cmd.GetButtonTextById(0); | ||
| Button2Text = cmd.GetButtonTextById(1); | ||
| Button3Text = cmd.GetButtonTextById(2); |
There was a problem hiding this comment.
Guard absent button labels before reading them
When callers use the default new MyMsgTextData("...") path, only buttons 0 and 1 exist, but this constructor still calls GetButtonTextById(2). That method dereferences the missing dictionary value, so the view model throws before the dialog can be shown for any dialog with fewer than three buttons; only read the label after the corresponding CanExecute check succeeds.
Useful? React with 👍 / 👎.
| DataContext = new MyMsgTextViewModel(data); | ||
| InitializeComponent(); | ||
| ShapeLine.StrokeThickness = ModBase.GetWPFSize(1d); | ||
| Loaded += Load; |
There was a problem hiding this comment.
Avoid using the legacy loader for data-backed dialogs
In the MyMsgTextData constructor path, myConverter is never assigned, but subscribing to the shared Load handler makes the loaded dialog execute Opacity = 0 and then dereference myConverter.IsWarn. The exception is caught after the opacity reset, so any data-backed dialog that does construct becomes invisible and never starts its show animation; use a loader that reads from the new data/view model or initialize the fields it depends on.
Useful? React with 👍 / 👎.
| else if (WaitingMyMsgBox.Count != 0) | ||
| { | ||
| // 没有弹窗,显示一个等待的弹窗 | ||
| frmMain.PanMsgBackground.Visibility = Visibility.Visible; | ||
| frmMain.PanMsg.Children.Add(new MyMsgText(WaitMyMsgData[0])); | ||
| WaitMyMsgData.RemoveAt(0); |
There was a problem hiding this comment.
Check the data-dialog queue before indexing it
This branch tests WaitingMyMsgBox.Count but then indexes WaitMyMsgData[0]. In the new data-dialog flow, a pending MyMsgTextData with no legacy dialog will never be shown, while a legacy dialog with an empty data queue will throw and be swallowed every time this method runs; the condition should be based on WaitMyMsgData.Count.
Useful? React with 👍 / 👎.
将部分控件转换为 MVVM 设计模式,以便实现控件解耦合
Work In Progress
Summary by Sourcery
为消息框控件引入 MVVM 风格的基础设施,并开始将其集成到现有的消息系统中。
新功能:
MyMsgTextData和ButtonContext模型以及MyMsgBoxCommand,以解耦的、基于命令的方式描述消息内容和按钮行为。MyMsgTextViewModel和ViewModelBase,以支持消息框文本对话框的 MVVM 绑定,包括按钮可见性和标签。ForEachIndexed,以简化在整个代码库中需要索引的迭代逻辑。增强内容:
MyMsgTextData的MyMsgText构造函数,并将其绑定到新的视图模型。ModMain中为待处理的MyMsgTextData实例添加存储和显示逻辑,与现有基于MyMsgBoxConverter的队列机制并行工作。MyMsgBoxTick逻辑,改用显式的Count检查来处理待显示的消息框。Original summary in English
Summary by Sourcery
Introduce MVVM-style infrastructure for message box controls and begin integrating it into the existing message system.
New Features:
Enhancements: