MaterialDesignInXAML / MaterialDesignInXamlToolkit

Google's Material Design in XAML & WPF, for C# & VB.Net.
http://materialdesigninxaml.net
MIT License
15.08k stars 3.42k forks source link

Move ColorToolViewModel to shared project #3624

Closed JLdgu closed 2 months ago

JLdgu commented 2 months ago

Both changes are to Demo projects

Keboo commented 2 months ago

Hey @JLdgu, while reviewing your PR, I'd suggest the following code changes:

👉 Code Suggestion for #3624

https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/pull/3624

I would set the default namespace for the shared project. So that the analyzer doesn't flag the files as not matching the project name.

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest >[Code Suggest](https://gitkraken.dev/redirect/aHR0cHM6Ly93d3cuZ2l0a3Jha2VuLmNvbS9zb2x1dGlvbnMvY29kZS1zdWdnZXN0P3NvdXJjZT1naXRodWIuY29tJm1lZGl1bT1wcl9jb21tZW50?source=PRSuggest&event=redirectToCodeSuggestPage&provider=github) liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR. _Join your team on [GitKraken](https://gitkraken.dev/redirect/aHR0cHM6Ly93d3cuZ2l0a3Jha2VuLmNvbT9zb3VyY2U9Z2l0aHViLmNvbSZtZWRpdW09cHJfY29tbWVudA==?source=PRSuggest&event=redirectToIndexPage&provider=github) to speed up PR review._
JLdgu commented 2 months ago

@Keboo On Discord I asked whether the namespace of the shared project should be MaterialDesignDemo.Shared in order to explicitly highlight shared code. @michaelmairegger said it should. If you want me to change the namespace should I included it in this PR or have this one merged and raise a new PR?

Keboo commented 2 months ago

@Keboo On Discord I asked whether the namespace of the shared project should be MaterialDesignDemo.Shared in order to explicitly highlight shared code. @michaelmairegger said it should. If you want me to change the namespace should I included it in this PR or have this one merged and raise a new PR?

ah sorry, I have been in the process of moving and have kept up. I am fine with it being a separate PR so we can get this one in.