Skip to content
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

Proposal: Port ColorPicker control from WinUI #72

Open
ShankarBUS opened this issue Apr 22, 2020 · 15 comments
Open

Proposal: Port ColorPicker control from WinUI #72

ShankarBUS opened this issue Apr 22, 2020 · 15 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ShankarBUS
Copy link
Contributor

I can't express how much I admire your work done for this library. You have ported and upgraded many controls for us to modernize our apps. ItemsRepeater, CommandBar, AutoSuggestBox are some amazing components to include in our
apps. Kudos to you @Kinnara

There are some cases where a user wants to change or choose a color(let's say the accent color of our apps, foreground color of a text control, etc.), in such cases we are demanded to provide a control for that. But the tranditional win32 color picker controls(eg. ColorDialog from WinForms) are out-of-date.

[ColorDialog from WinForms]

colordialog1

We can use other libs (wpftoolkit for instance) for that but we need a ColorPicker with Fluent UI.

It would be great if you can port it from WinUI

[ColorPicker from WinUI]

ColorPicker

This will enable a much more consistent environment for our apps.

This is not a mandatory need but would be great if added. Also consider @XamDR's proposal (#61) these additions would be great for text editor based apps. But unlike @XamDR's proposal, porting ColorPicker is much easier since it is open sourced in the WinUI repo. (Pleaseeeeeee 🙏)

@Kinnara
Copy link
Owner

Kinnara commented Apr 22, 2020

Thank you for the suggestion. The WinUI ColorPicker has been on my radar for a while, and will be one of the priorities post v0.9 release.

@ShankarBUS ShankarBUS changed the title [Feature Proposal] Port ColorPicker control from WinUI Proposal: Port ColorPicker control from WinUI Apr 26, 2020
@Kinnara Kinnara added the enhancement New feature or request label Jun 18, 2020
@maxkatz6
Copy link

maxkatz6 commented Sep 1, 2020

@Kinnara just for case, I started working on that ColorPicker from WinUI, but for Avalonia.
Currently I don't have enough time to complete it, but source code could be useful for you or somebody else.
AvaloniaUI/Avalonia#4179

@robloo
Copy link

robloo commented Sep 1, 2020

@maxkatz6 As mentioned, I'm doing a port for the Uno Platform too. Will make sure to check against your code base for inconsistencies. This would be another candidate for a base version of a WPF port.

Separately... We will have 3 different C# versions of the control: Avalonia, WPF, Uno Platform. We probably need to get creative on automating changes so the port from C++ to C# occurs once and then the small changes for the various C# flavors (Avalonia/WPF) get applied on top of a pure UWP port.

@ShankarBUS
Copy link
Contributor Author

Hey @robloo and @maxkatz6, that's awesome people!

Porting from either UWP/WinUI/Uno platform (c++/C#) will be relatively easier than from Avalonia (since it has a different styling system than previous).

If you did the Uno platform part, it will be much easier to work on (just UWP to WPF, I have done some porting WCT controls and I have some experience on that).

But porting directly from WinUI (c++) will be the best option (IMO, since you will do platform specific code for Avalonia/Uno platform. You may have to do some fills and stuffs). It will be more consistent with the official one (@Kinnara is skilled in that subject).

@robloo, The new color picker button you made for WCT could be added to the ModernWpfCommunityToolkit.

The author of this lib and you both could interact and share your work together to port it from c++ to C# (I see @maxkatz6 already did the work for Avalonia but the implementation will be specific to Avalonia and if @robloo did it for Uno platform it will be a different strain of it).

I watch your contributions on github and I have to admit that I'm a fan of you all 😁.

@robloo
Copy link

robloo commented Sep 1, 2020

I think the hardest part is actually porting from C++ to C#. There will be two versions to choose from soon though: Uno or Avalonia. In developing for Uno I am also going to make a stand-alone version that runs on UWP. That is probably the easiest to use for the WPF base.

So bottom line, both @maxkatz6 and I will have ported directly from C++ to C#. I certainly wouldn't recommend going through that work again. It is by far easier to just update the styling from Avalonia or, even easier, to make the transition from UWP.

The ColorPickerButton isn't quite stable yet. Definitely would be great to see this in WPF though! Happy to help when the time comes.

@Kinnara
Copy link
Owner

Kinnara commented Sep 13, 2020

A big thank you to you all. I'm not sure if there are license issues porting from Uno, but porting from Avalonia should be possible. It's definitely fastest to port from another C# code base, while @ShankarBUS's point about consistency with WinUI is also valid. We may try a compromise: porting the platform-specific part from WinUI, and the rest from Avalonia. Anyhow, I believe it's time to start the work. It likely won't be super fast due to a lot of personal matters, but shouldn't take too long either thanks to the work you guys have done.

@robloo
Copy link

robloo commented Sep 13, 2020

@Kinnara As the author of the ColorPicker port in Uno, I would be happy to provide you a zip file of the code licensed as MIT. My initial port was a conversion straight to C# as a stand-alone project to make sure things worked. Then I put it into Uno. I can give you the original C# code after I clean it up a bit. As the author its (1) within my power to license this as MIT (2) this version of the code would pre-date the one given to Uno under the Apache license.

@Kinnara
Copy link
Owner

Kinnara commented Sep 13, 2020

@robloo That's very kind of you! My email is [email protected] if you prefer to send it that way. Thank you again.

@Kinnara Kinnara self-assigned this Sep 13, 2020
@robloo
Copy link

robloo commented Sep 13, 2020

@Kinnara I have no problem sharing the code here! I just didn't want to create a new repo for this.

I've packaged up all the code and made some updates where appropriate. Please note the following:

  • Remember this is a UWP port -- so several little things will need to change for WPF (as you know well)
  • All namespaces have 'Test' after them and should be named to something that makes sense for WPF
  • The port preserves the WinRT-style async code -- which will need to be re-written to standard C# async/Task patterns. This was quickly done in the Uno port code-base but without cancellation support.
  • Consider removing the second method of generating a spectrum using the SpectrumBrush (it's commented out in this version and missing SpectrumBrush.cs entirely). That is, unless you want to port this to WPF. As it uses Composition API's from UWP it wasn't ported but would be possible to use a different approach in WPF.
  • None of the strings work -- you'll have to copy them over yourself and use a new ResourceAccessor. You'll notice some other helper stubs as well that will need to be removed. Note that the WinUI localized strings do not use .net formatting ("{0}").
  • You might want to create a new List structure that has direct access to the underlying array. This will avoid a copy when creating the WritableBitmap. The code for that is within the Uno port if you ever wanted to take a look.

Overall, this is a very-close port of the WinUI version of the color picker and I only deviated where there was no other choice. The directory/class structure also very closely follows the WinUI code base. @maxkatz6 Port is very good/high-quality though and I referenced it to double check a few things. The only benefits of using this is it more closely matches WPF since it is written in UWP.

I also recommend reading this: https://github.com/microsoft/microsoft-ui-xaml/blob/308e4b551d05a87eb10d674d4b1feb8e34ba0111/dev/ColorPicker/readme.md

If you have any questions feel free to ask! I'm sure one of us can save you some time if you run into troubles. Otherwise, you are doing a lot of great work here. I would certainly use your project if I was using WPF more often these days. Stay safe and I hope all will be well soon!

License: MIT (code base authored by myself before contributing a later iteration of it to Uno)
Base WinUI Git Version: winui_git_version_3c4a564f6bfca81552ebbc54652d2f4e00c3bc22

ColorPicker2.zip

@Kinnara
Copy link
Owner

Kinnara commented Sep 14, 2020

@robloo Really grateful for the source code and all the notes/advice/additional information. I've heard someone say that the greatest respect for food is to cook them as delicious as possible, and I believe this also applies to the help you and others have provided.

@Kinnara Kinnara added this to the 0.9.3 milestone Sep 23, 2020
@ShankarBUS
Copy link
Contributor Author

Any updates @Kinnara?

It's been more than a month 😅. I understand that you have other things to focus on. Just give us some information on this, please?

@Kinnara
Copy link
Owner

Kinnara commented Oct 30, 2020

Any updates @Kinnara?

It's been more than a month 😅. I understand that you have other things to focus on. Just give us some information on this, please?

Work has started but going slowly due to limited time. Starting next week I expect to have a little (literally) more free time. Let's set a deadline at the end of November and try to hit it!

@Kinnara Kinnara modified the milestones: 0.9.3, 0.9.4 Dec 6, 2020
@MelbourneDeveloper
Copy link

@robloo nice to see you over here as well. How is the progress with color picking controls for Avalonia?

@robloo
Copy link

robloo commented Jan 24, 2021

Might be best to talk about that over on the Avalonia github issue. @maxkatz6 has the Avalonia port so he will have to provide an update. We both ended up porting separately from c++ to c#. UWP and Avalonia have enough differences, and the UWP port needs to maintain very high code similarity with WinUI especially for Uno (in order to keep fixes/updates synced).

My offer stands to contribute the ColorPickerButton modifications done for the Windows community toolkit if/when Avalonia is ready though.

@maxkatz6
Copy link

maxkatz6 commented Jan 25, 2021

@MelbourneDeveloper I pushed my ColorPicker, that ported from WinUI, to the NuGet.
https://www.nuget.org/packages/AvaloniaWinUI.ColorPicker/0.10.0-preview-g35ec5e0d69

You can see source code here:
https://github.com/maxkatz6/AvaloniaWinUI.ColorPicker

It lacks some docs/readme/ci, but works properly as far as I tested.

Also there is another color picker controls for Avalonia:
https://github.com/egorozh/Egorozh.ColorPicker
https://github.com/arklumpus/AvaloniaColorPicker
https://github.com/wieslawsoltes/ThemeEditor (color picker only on nuget)

@Kinnara Kinnara modified the milestones: 0.9.4, 0.9.5 Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants