On 29/02/20 10:46 AM, commits at source.squeak.org wrote:
>    Morph subclass: #NewColorPickerMorph
> + 	instanceVariableNames: 'target setColorSelector hsvaMorph colorPresenter isPicking'
> - 	instanceVariableNames: 'target setColorSelector hsvaMorph colorPresenter'

is isPicking really required? Why introduce a mode?

>    ----- Method: NewColorPickerMorph>>colorSelected: (in category 'model') -----
>    colorSelected: aColor
> + 	self isPicking ifFalse: [
> + 		self targetColor: aColor].
> - 	self targetColor: aColor.

targetColor is getting set regardless of isPicking here.

> + ----- Method: NewColorPickerMorph>>pickColor (in category 'picking') -----
> + pickColor
> + 	
> + 	| selectedColor |
> + 	[
> + 		| previousColor |
> + 		previousColor := self selectedColor.
> + 		selectedColor := self pickColorFromDisplay.
> + 		selectedColor ifNil: [^ self selectedColor: previousColor].
> + 	] ensure: [
> + 		self isPicking: false].
> + 	self selectedColor: selectedColor.!

Could this be simplified to :

  self pickColorFromDisplay ifNotNil: [:pickedColor | self 
selectedColor: pickedColor ].

Regards .. Subbu