Various Colorpicker Issues #1876
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1876 +/- ##
======================================
Coverage 100% 100%
======================================
Files 414 414
Lines 8642 8653 +11
Branches 1279 1281 +2
======================================
+ Hits 8642 8653 +11
Continue to review full report at Codecov.
|
| @@ -123,16 +122,20 @@ export class SkyColorpickerInputDirective | |||
| const element = this.elementRef.nativeElement; | |||
|
|
|||
| this.renderer.setElementClass(element, 'sky-form-control', true); | |||
| this.skyColorpickerInput.initialColor = this.initialColor; | |||
| this.skyColorpickerInput.initialColor = this.initialColor ? this.initialColor : SKY_COLORPICKER_DEFAULT_COLOR; | |||
There was a problem hiding this comment.
Would it be possible to move this Default color to the get method for initialColor?
return this._intialColor || SKY_COLORPICKER_DEFAULT_COLOR;
We often do this for default values so that when no value is assigned yet the default is always returned.
There was a problem hiding this comment.
Actually yes..... I made that change in an earlier version of the final product and hadn't noticed that things had cleaned up so that I could revert it back. Done.
| this._onChange(newColor); | ||
| }); | ||
|
|
||
| this.skyColorpickerInput.setColorFromString(this.initialColor); | ||
| this.skyColorpickerInput.setColorFromString(this.skyColorpickerInput.lastAppliedColor); |
There was a problem hiding this comment.
I think this might be a little more straightforward to read if this and line 126 were set directly equal to this.initialColor. Unless there is some change that can happen after line 125 that I'm not noticing.
There was a problem hiding this comment.
That boiled down to the fact that I wasn't doing the fallback getter and not wanting to have to do the ternary every time. Now that that has changed I can go back to using the initialColor getter.
|
Documentation issue: #1893 |
#1872
#1617
Fixed these things
Colorpicker no longer closes if you click off the dropdown which contains the opened picker (Verified with Todd Roberts on the expected behavior here)
Clicking "Close" on the picker now reverts the picker back to the color that was previously chosen (or given if initially set via code).
The reset button now reverts the picker to the color given via code to be the initial color (or white if none was originally given) - previously this button always changed the color to white.
The picker now emits a new selectedColorApplied event upon a user clicking the "Apply" button. The selectedColorChanged event will still emit on any change to an open picker as it always has.