feat(stage-1): retire WinForms SimulateForm/Control and WelcomeForm#7
Conversation
….UI to WPF dialogs - Delete WelcomeForm (unused), SimulateForm + SimulateControl (WinForms) - Move HooverTimer from SAM.Core.UI to SAM.Core.Mollier.UI (only used there) - Drop RibbonWinForms NuGet from SAM.Analytical.UI.csproj - Remove UseWindowsForms from SAM.Core.UI.csproj; add System.Drawing.Common 7.0.0 - Migrate UIJSAMObject dialogs to WPF MessageBox + Microsoft.Win32.SaveFileDialog - Add UpdateConstructionLayersByPanelType checkbox to WPF SimulateControl - Wire WPF Modify/Simulate.cs second overload to WPF SimulateWindow + WindowInteropHelper Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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: 350cd5a6cb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| projectName = simulateWindow_Path.ProjectName; | ||
| outputDirectory = simulateWindow_Path.OutputDirectory; |
There was a problem hiding this comment.
This overload now accepts ProjectName and OutputDirectory directly from SimulateWindow without validating them, while the removed WinForms SimulateForm previously blocked empty project names and non-existent output directories before returning OK. In the EnergySimulation path (which calls this overload), users can now proceed with invalid values and trigger writes to an unintended working directory or downstream conversion failures when creating the TBD file.
Useful? React with 👍 / 👎.
|
|
||
| <CheckBox x:Name="checkBox_CreatePartL" Content="Create Part L" HorizontalAlignment="Left" Margin="30,450,0,0" VerticalAlignment="Top"/> | ||
|
|
||
| <CheckBox x:Name="checkBox_UpdateConstructionLayersByPanelType" Content="Update Construction Layers By Panel Type" HorizontalAlignment="Left" Margin="30,473,0,0" VerticalAlignment="Top" IsChecked="True"/> |
There was a problem hiding this comment.
The new Update Construction Layers By Panel Type checkbox is now visible in the shared WPF simulate UI, but the primary Simulate(this UIAnalyticalModel) path still hardcodes updateConstructionLayersByPanelType = true and never reads this control. That makes user input ineffective in the main workflow and creates inconsistent behavior versus the newly updated secondary overload.
Useful? React with 👍 / 👎.
- Add SPDX header to MollierControl.cs (fixes SPDX CI failure) - Primary Simulate(UIAnalyticalModel) overload: read UpdateConstructionLayersByPanelType from SimulateWindow instead of hardcoding true - Second Simulate(AnalyticalModel, path) overload: add projectName and outputDirectory validation guards before starting simulation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e7012fba5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SimulateWindow simulateWindow_Path = new SimulateWindow(); | ||
| simulateWindow_Path.ProjectName = System.IO.Path.GetFileNameWithoutExtension(path); | ||
| simulateWindow_Path.OutputDirectory = System.IO.Path.GetDirectoryName(path); | ||
| simulateWindow_Path.WeatherData = weatherData; |
There was a problem hiding this comment.
This overload now opens SimulateWindow without setting legacy defaults beyond path/weather, so accepting the dialog can silently change simulation behavior compared with the removed WinForms flow. In particular, the previous SimulateForm defaulted Sizing to enabled, while the WPF control starts unchecked unless explicitly set; this can disable sizing runs and alter produced results for the same input model when users just click OK.
Useful? React with 👍 / 👎.
| public bool UpdateConstructionLayersByPanelType | ||
| { | ||
| get | ||
| { | ||
| return checkBox_UpdateConstructionLayersByPanelType.IsChecked != null && checkBox_UpdateConstructionLayersByPanelType.IsChecked.HasValue && checkBox_UpdateConstructionLayersByPanelType.IsChecked.Value; |
There was a problem hiding this comment.
The new UpdateConstructionLayersByPanelType UI option is exposed and read during simulation, but it is not included in the SimulateOptions round-trip used by ActiveSetting (the control still serializes/deserializes the older option set). As a result, users who change this checkbox lose their choice the next time the dialog opens, which can re-enable layer updates unexpectedly.
Useful? React with 👍 / 👎.
- Path-based Simulate overload now sets Sizing=true and UpdateConstructionLayersByPanelType=true on the new WPF SimulateWindow, matching the WinForms SimulateForm defaults that were lost during the WinForms → WPF swap. Prevents silent regression where clicking OK without touching the Sizing checkbox would skip sizing. - Persist UpdateConstructionLayersByPanelType through SimulateOptions: added property (default true), copy constructor entry, JSON read/write, and the SimulateControl.GetSimulateOptions/SetSimulateOptions round-trip. Users' checkbox state now survives ActiveSetting save/restore. Codex findings on PR #7 (already merged): P1 Sizing default regression and P2 UpdateConstructionLayersByPanelType persistence. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
Stage 1 of the WinForms → WPF migration in SAM_UI. After this PR, only
SAM.Core.Mollier.UIremains on WinForms.WelcomeFormfromSAM.Core.UI(no references anywhere)SimulateForm+SimulateControl(WinForms) fromSAM.Analytical.UI— WPF twin already exists and is more completeHooverTimerfromSAM.Core.UI→SAM.Core.Mollier.UI(only user); detected as rename by gitRibbonWinForms5.1.0-beta NuGet fromSAM.Analytical.UI(ribbon lives in WPFAnalyticalWindow.xaml)UseWindowsForms=truefromSAM.Core.UI.csproj; add explicitSystem.Drawing.Common 7.0.0UIJSAMObjectdialogs →System.Windows.MessageBox+Microsoft.Win32.SaveFileDialogUpdateConstructionLayersByPanelTypecheckbox to WPFSimulateControl(was missing vs WinForms version)Modify/Simulate.cssecond overload to use WPFSimulateWindow+WindowInteropHelperCoordinated PR: SAM_Revit_UI
sam-revit-ui/wpf-simulate-elementhostmust be merged after this one.Test plan
dotnet buildsucceeds with 0 errorsUseWindowsFormsinSAM.Core.UI.csprojafter merge.csfiles🤖 Generated with Claude Code