-
-
Notifications
You must be signed in to change notification settings - Fork 72
Project Maintenance: Reorganization and Documentation Improvement #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Replace legacy .sln format with modern .slnx format and relocate from src/ to project root
Restrict 2-space indentation to XML-based files (xml, slnx, fsproj, csproj, xaml) Install dotnet tool fantomas to handle F# formatting
Include .editorconfig, .gitignore, and LICENSE.md in solution folder
Format all F# source files in src/Elmish.WPF/ Format all F# test files in src/Elmish.WPF.Tests/ Format all F# sample project files Standardize code style and indentation
Improve code documentation comments of Elmish.WPF project.
Move all dynamic binding samples to src/Samples/Dynamic/ Move typed ViewModel samples to src/Samples/Typed/ Relocate test projects to tests/ folder Update .slnx file to reflect new project structure Add README explaining both binding approaches This reorganization makes it clearer for users to understand and choose between the dynamic bindings() approach and the typed ViewModelBase approach.
Update files reflecting the changes made in the project structure.
Create a new sln file. Update the workflow files to point to the sln file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This is a comprehensive maintenance PR that adds new typed samples to the Elmish.WPF framework and reorganizes the existing sample structure. The PR introduces a "Typed" approach to data binding while preserving the existing "Dynamic" approach, creating a clear distinction between the two paradigms.
Key changes:
- Reorganizes samples into Dynamic and Typed folders to distinguish binding approaches
- Adds new typed sample applications that use strongly-typed ViewModels
- Creates comprehensive documentation explaining both binding approaches
Reviewed Changes
Copilot reviewed 247 out of 405 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Samples/README.md | Documents the two binding approaches and sample organization |
| src/Samples/Typed/*/Program.fs | New typed samples using ViewModelBase inheritance |
| src/Samples/Dynamic/* | Reorganized existing samples into Dynamic folder |
| src/Samples/Typed//.xaml.cs | Code-behind files for new typed samples |
| src/Samples/Typed//.csproj | Project files for new typed C# projects |
| <StackPanel Width="300"> | ||
| <TextBlock Text="Form 1" FontSize="18" FontWeight="Bold" HorizontalAlignment="Center" Margin="0,0,0,5" /> | ||
| <TextBox Text="{Binding Text, UpdateSourceTrigger=PropertyChanged}" TextWrapping="Wrap" AcceptsReturn="True" Height="80" Margin="0,5,0,5" /> | ||
| <TextBox Text="{Binding Text, UpdateSourceTrigger=PropertyChanged}" TextWrapping="Wrap" AcceptsReturn="True" |
Copilot
AI
Jul 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This line is being formatted but has inconsistent casing with 'UpdateSourceTrigger' using PascalCase while 'TextWrapping' and 'AcceptsReturn' use PascalCase consistently. This is correct XAML syntax.
|
|
||
| private void UIElement_IsVisibleChanged(object sender, DependencyPropertyChangedEventArgs e) | ||
| { | ||
| if (e.NewValue is bool b && b) _ = AssociatedObject.Focus(); |
Copilot
AI
Jul 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using pattern matching with a simple boolean comparison 'b && b' can be simplified to just check if the value is true: 'e.NewValue is true'
| if (e.NewValue is bool b && b) _ = AssociatedObject.Focus(); | |
| if (e.NewValue is true) _ = AssociatedObject.Focus(); |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\..\Elmish.WPF\Elmish.WPF.fsproj"/> |
Copilot
AI
Jul 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project reference path uses backslashes which may cause issues on non-Windows systems. Consider using forward slashes for cross-platform compatibility.
| <ProjectReference Include="..\..\..\Elmish.WPF\Elmish.WPF.fsproj"/> | |
| <ProjectReference Include="../../../Elmish.WPF/Elmish.WPF.fsproj"/> |
Update README.md to emphasize both Dynamic and Typed ViewModel approaches Add comprehensive guidance on when to choose each binding approach Revise getting started section with dual-path examples for both approaches Add FAQ entries for approach selection and migration between Dynamic/Typed Significantly expand REFERENCE.md statically-typed ViewModels section Document proper TwoWay binding patterns for typed ViewModels (getter/setter) Add new "Choosing Your Binding Approach" section with comparison table Update all sample references to reflect new Dynamic/Typed folder organization Add comprehensive examples for typed bindings, validation, and sub-models This update ensures documentation accurately reflects the complete set of typed samples and the reorganized project structure, providing users with clear guidance on both binding approaches and migration paths. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add a sample how to create a simple module based navigation using typed view models. Fix solution file structure.
|
Please update PR #591 to include fantomas. Also please refrain from fully comprehensive PR's that touch 10000's of lines of code and do more than one thing. The reorganization should be separate from the added examples (and that separate from any feature changes) so we can reasonably review all of the changes by hand. |
marner2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not change 40,000 lines in a single pull request without being extremely clear about the single thing you are doing. This PR has multiple things going on, many of which can be split out into smaller, much more focused PRs. Additionally as stated earlier it contains changes that are currently pending in another PR.
|
I move to close this PR. The requester can then make smaller, more atomic PRs if he so chooses. |
To improve the future development of the project all the samples have been ported to the typed view-model-bindings.
The documentation and reference documents have been updated to reflect the changes.
The code has been reorganized.
testsandbenchmarksare moved to thetestsfolder.srcfolder holds theElmish.WPFandSamplesprojects.slnandslnx.fantomas