Color picker does not retain bound value from component

[Blackbaud-ScottFreeman]

Expected behavior

You should be able to bind a property to the color picker and have it retain that value if the user clicks the color picker and closes it without making any changes.

Actual behavior

If you follow the steps above, the color picker is reset to white.

Plunker

https://plnkr.co/edit/TOXjSeIBgmFnp0ZV6meF?p=preview

[Blackbaud-PaulCrowder]

The current behavior is not really ideal, but for what it's worth, you must set the initialColor property as well as ngModel in order to get the behavior you want. Here's a Plunker that demonstrates this technique. We'll look into changing this behavior in the future so it's more intuitive.

https://plnkr.co/edit/P5vme9Pv6Ypobv6MR6A1?p=preview

[Blackbaud-ScottFreeman]

I thought I had a plunker that showed why this wouldn't work, but I believe that I need to go back and figure out why we aren't actually able to do this. I tried hard finding a workaround for this issue yesterday and I kept getting stuck because the value being loaded was not being used when saving. I'll try to reproduce that issue in plunker.

[Blackbaud-ScottFreeman]

We may actually be able to use the initialColor in our scenario. The issue with saving might simply be due to the fact that it is often a mystery whatever type your bound property is going to be. Is it a string or is it an instance of SkyColorpickerOutput? Here's a plunker that shows what I mean.
https://plnkr.co/edit/4wiXNDVBx7lHGWyCP8yb?p=preview
If you don't touch the color picker, then color1 will be the hex string that you initialize it with. If you open the color picker dialog and then click Apply, then color1 will be of type SkyColorpickerOutput. This kind of uncertainty makes it difficult to use the color picker at times. To be honest, I don't know why the bound property isn't always a string of whatever format you said you wanted to use. I also don't see the point of the initialColor property. If you want to initialize the color picker to a certain color, then initialize the property you bind it to with that color.

[Blackbaud-PaulCrowder]

I spoke with @Blackbaud-JaminQuimby who originally authored this component and we agreed that initialColor should probably be removed and just let ngModel be the way to set the value. We'll see about getting this in next week.

[Blackbaud-SteveBrush]

I spent the good part of 3 days attempting to refactor the colorpicker to use only ngModel, but I couldn't get around breaking changes due to @Blackbaud-ScottFreeman's comment about the variable's type (string vs SkyColorpickerOutput). For example, if a user provides a string to the ngModel attribute, it gets processed as a SkyColorpickerOutput, which causes the linter to fail.

I recommend we leave it as it is. I've addressed the "default value" issue in this PR: #1227. If we want to refactor the colorpicker, I recommend we either wait until UX v.3, or create a Colorpicker2 component.

[Blackbaud-ScottFreeman]

Is there a reason why we have to keep SkyColorpickerOutput around besides causing a breaking change if you remove it? I'm pretty sure that this component is barely used at the moment and people who do use it would be glad to get rid of SkyColorpickerOutput. The simple fact that that your property could be either a string or a SkyColorpickerOutput when it comes time to save data is definitely going to cause bugs, especially if people use the any type for their property. What will happen is that they will try accessing the color with code similar to this.color.hex, and this will work many times except for those instances where this.color is a string and this.color.hex returns undefined leaving the developer wondering why the color saves sometimes but not others. Note that I have similar feelings about how time picker works, but that's a different component/can of worms.

[Blackbaud-SteveBrush]

@Blackbaud-ScottFreeman Agreed. I'll lean into @Blackbaud-PaulCrowder's response, but I'm assuming we'll need to consider a refactor for colorpicker (and perhaps datepicker) for the v.3 release candidate.

For reference: https://github.com/blackbaud/skyux2/projects/9