-
Notifications
You must be signed in to change notification settings - Fork 22
Simplify and modernize WPF sample #368
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: main
Are you sure you want to change the base?
Conversation
Add the `ThemeMode="System"` attribute in `App.xaml` and simplify the resource definition. Delete the `BooleanToImageConverter.cs` file and its content. Remove the `completed.png` and `incomplete.png` image files. Replace the image display logic with a `TextBlock` in `MainWindow.xaml`, and update the `TextBox` of `NewTodoItemTitle` to support instant updates. Remove the references to the deleted images in the project file while keeping other settings unchanged.
Will get to this after I have done the release today - apologies for the delay. |
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 PR simplifies and modernizes the WPF sample by streamlining resource definitions, eliminating unused image assets, updating UI elements, and adding keyboard support.
- Removed image resource references and the BooleanToImageConverter.
- Updated MainWindow.xaml to use TextBlock icons instead of images and refined the layout.
- Set the ThemeMode to "System" in App.xaml and added Enter shortcut support via IsDefault on the Add button.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
samples/todoapp/TodoApp.WPF/TodoApp.WPF.csproj | Removed image resource definitions and references to deleted images |
samples/todoapp/TodoApp.WPF/MainWindow.xaml | Replaced image-based icon rendering with TextBlock controls; adjusted layout and added Enter shortcut support via IsDefault |
samples/todoapp/TodoApp.WPF/Converters/BooleanToImageConverter.cs | Deleted obsolete BooleanToImageConverter |
samples/todoapp/TodoApp.WPF/App.xaml | Added ThemeMode="System" attribute and simplified resource definition |
<DataTemplate x:Key="CompletedIconTemplate"> | ||
<Image Width="16" Height="16" Source="{Binding Path=IsComplete, Converter={StaticResource BooleanToImageConverter}}" /> | ||
<Grid> |
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.
Both TextBlock elements inside the Grid may be visible simultaneously when IsComplete is false. Consider adding a Visibility binding (e.g., using an inverse BooleanToVisibility converter) on the second TextBlock so that only the appropriate icon is shown based on the task's completion state.
Copilot uses AI. Check for mistakes.
Overall, I like the new styling. However, take a look at this screen shot: The border around each section is too thick and doesn't encompass the complete add task (there is no bottom border) - probably hidden by the padding in the MainWindow. That is not a problem. However, in dark mode, the highlighting is a definite problem. I cannot read that test issue. I also could not "uncomplete" the issue. I had to click on refresh to get rid of the highlight. Once those issues are solved, I think this is good to go. |
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.
Per my other comment - overall good, but needs some tweaks for dark mode.
Remove the old CompletedIconTemplate, and introduce a new TaskItemTemplate. Change the layout from StackPanel to Grid to better organize the interface elements. Update the task list to use the new template and optimize the layout of the bottom input area, which includes a TextBox and two buttons.
ThemeMode="System"
attribute inApp.xaml
and simplify the resource definition.BooleanToImageConverter.cs
file and its content. Remove the references to the deleted images in the project file.completed.png
andincomplete.png
image files. Replace the image display logic with aTextBlock
inMainWindow.xaml
.