💄 more color picker cleanup

src/vs/editor/contrib/colorPicker/browser/colorPickerModel.ts

@@ -7,54 +7,57 @@ import Event, { Emitter } from 'vs/base/common/event';
 import { ColorPickerWidget } from 'vs/editor/contrib/colorPicker/browser/colorPickerWidget';
 import { Color, RGBA } from 'vs/base/common/color';
 import { IColorFormatter } from 'vs/editor/contrib/colorPicker/common/colorFormatter';
-import { IModel } from 'vs/editor/common/editorCommon';
-import { Range, IRange } from 'vs/editor/common/core/range';
 
 export class ColorPickerModel {
-	public widget: ColorPickerWidget;
 
-	public saturationSelection: ISaturationState;
-	public originalColor: Color;
-	public formatters: IColorFormatter[];
-	public saturation: number; // [0-1]
-	public value: number; // [0-1]
+	widget: ColorPickerWidget;
+	saturationSelection: ISaturationState;
+	originalColor: Color;
+	formatters: IColorFormatter[];
+	saturation: number; // [0-1]
+	value: number; // [0-1]
 
 	private _color: Color;
-	private _selectedColor: string;
 	private _opacity: number;
 	private _hue: number;
 
 	private _formatter: IColorFormatter;
+	get formatter(): IColorFormatter { return this._formatter; }
 
-	private _colorRange: Range;
-	private _colorModelIndex: number;
+	private formatterIndex: number;
 
 	private _onDidChangeColor = new Emitter<Color>();
 	readonly onDidChangeColor: Event<Color> = this._onDidChangeColor.event;
 
+	private _onDidChangeFormatter = new Emitter<IColorFormatter>();
+	readonly onDidChangeFormatter: Event<IColorFormatter> = this._onDidChangeFormatter.event;
+
 	constructor(
 		color: Color,
 		formatter: IColorFormatter,
-		availableFormatters: IColorFormatter[],
-		private editorModel: IModel,
-		range: IRange
+		availableFormatters: IColorFormatter[]
 	) {
-		this.formatters = [];
-		this._colorModelIndex = 0;
+		if (availableFormatters.length === 0) {
+			throw new Error('Color picker needs formats');
+		}
+
+		this.formatterIndex = 0;
 
 		this.originalColor = color;
 		this.formatters = availableFormatters;
 		this._formatter = formatter;
-		this.color = color;
+		this._color = color;
 		this.hue = color.hsla.h;
 		this.saturation = color.hsla.s;
 		this.value = color.hsva.v;
-		this._colorRange = new Range(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn);
 	}
 
-	public set color(color: Color) {
+	set color(color: Color) {
+		if (this._color.equals(color)) {
+			return;
+		}
+
 		this._color = color;
-		this._onDidChangeColor.fire(color);
 
 		const alpha = color.rgba.a;
 		if (!this._opacity) {
@@ -64,84 +67,51 @@ export class ColorPickerModel {
 		this.value = color.hsva.v;
 
 		if (!this._formatter.canFormatColor(color)) {
-			this.nextColorMode();
-		} else {
-			this.selectedColorString = this._formatter.formatColor(this._color);
-		}
-	}
-
-	public get color(): Color {
-		return this._color;
-	}
-
-	public set selectedColorString(colorString: string) {
-		if (this._selectedColor === colorString) {
-			return;
+			this.selectNextColorFormat();
 		}
-		this._selectedColor = colorString;
-
-		if (this.widget && this.widget.body) {
-			this.widget.body.fillOpacityOverlay(this._color);
-
-			this.editorModel.pushEditOperations([], [{
-				identifier: null,
-				range: this._colorRange,
-				text: colorString,
-				forceMoveMarkers: false
-			}], () => []);
 
-			this._colorRange = this._colorRange.setEndPosition(this._colorRange.endLineNumber, this._colorRange.startColumn + colorString.length);
-		}
+		this._onDidChangeColor.fire(color);
 	}
 
-	public get selectedColorString() {
-		return this._selectedColor;
+	get color(): Color {
+		return this._color;
 	}
 
-	public set hue(hue: number) {
+	set hue(hue: number) {
 		this._hue = hue;
 		if (this.widget && this.widget.body) {
 			this.widget.body.saturationBox.fillSaturationBox();
 		}
 	}
 
-	public get hue(): number {
+	get hue(): number {
 		return this._hue;
 	}
 
-	public set opacity(opacity: number) {
+	set opacity(opacity: number) {
 		this._opacity = opacity;
 
 		const rgba = this._color.rgba;
 		this.color = new Color(new RGBA(rgba.r, rgba.g, rgba.b, opacity * 255));
 	}
 
-	public get opacity(): number {
+	get opacity(): number {
 		return this._opacity;
 	}
 
-	public nextColorMode() {
-		if (this.formatters.length === 0) {
-			return;
-		}
-
-		this._colorModelIndex++;
-
-		if (this._colorModelIndex === this.formatters.length) {
-			this._colorModelIndex = 0;
-		}
-
-		this._formatter = this.formatters[this._colorModelIndex];
+	selectNextColorFormat(): void {
+		this.formatterIndex = (this.formatterIndex + 1) % this.formatters.length;
+		this._formatter = this.formatters[this.formatterIndex];
 
 		if (!this._formatter.canFormatColor(this._color)) {
-			this.nextColorMode();
-		} else {
-			this.selectedColorString = this._formatter.formatColor(this._color);
+			return this.selectNextColorFormat();
 		}
+
+		this._onDidChangeFormatter.fire(this._formatter);
 	}
 }
 
 export class ISaturationState {
-	public x: number;
-	public y: number;
+	x: number;
+	y: number;
 }
\ No newline at end of file

src/vs/editor/contrib/colorPicker/browser/colorPickerWidget.ts

@@ -32,14 +32,19 @@ export class ColorPickerHeader extends Disposable {
 		const colorBox = dom.append(this.domNode, $('.original-color'));
 		colorBox.style.backgroundColor = Color.Format.CSS.format(this.model.originalColor);
 
-		this._register(dom.addDisposableListener(this.pickedColorNode, dom.EventType.CLICK, () => this.model.nextColorMode()));
+		this._register(dom.addDisposableListener(this.pickedColorNode, dom.EventType.CLICK, () => this.model.selectNextColorFormat()));
 		this._register(model.onDidChangeColor(this.onDidChangeColor, this));
-		this.onDidChangeColor(this.model.color);
+		this._register(model.onDidChangeFormatter(this.onDidChangeFormatter, this));
+		this.onDidChangeColor();
 	}
 
-	private onDidChangeColor(color: Color) {
-		this.pickedColorNode.textContent = this.model.selectedColorString;
+	private onDidChangeColor(): void {
 		this.pickedColorNode.style.backgroundColor = Color.Format.CSS.format(this.model.color);
+		this.onDidChangeFormatter();
+	}
+
+	private onDidChangeFormatter(): void {
+		this.pickedColorNode.textContent = this.model.formatter.formatColor(this.model.color);
 	}
 }
 
@@ -65,6 +70,12 @@ export class ColorPickerBody extends Disposable {
 		this.drawHueStrip();
 
 		this.registerListeners();
+		this._register(model.onDidChangeColor(this.onDidChangeColor, this));
+	}
+
+	private onDidChangeColor(): void {
+		const { r, g, b } = this.model.color.rgba;
+		this.opacityOverlay.style.background = `linear-gradient(to bottom, rgba(${r}, ${g}, ${b}, 1) 0%, rgba(${r}, ${g}, ${b}, 0) 100%)`;
 	}
 
 	layout(): void {
@@ -179,7 +190,7 @@ export class ColorPickerBody extends Disposable {
 		this.opacityStrip = $('.strip.opacity-strip');
 		dom.append(this.domNode, this.opacityStrip);
 		this.opacityOverlay = $('.opacity-overlay');
-		this.fillOpacityOverlay(this.model.color);
+		this.onDidChangeColor();
 		dom.append(this.opacityStrip, this.opacityOverlay);
 
 		this.opacitySlider = new Slider(this.opacityStrip);

src/vs/editor/contrib/hover/browser/modesContentHover.ts

@@ -27,7 +27,7 @@ import { ColorPickerWidget } from 'vs/editor/contrib/colorPicker/browser/colorPi
 import { ColorDetector } from 'vs/editor/contrib/colorPicker/browser/colorDetector';
 import { IColorFormatter, ColorFormatter, CombinedColorFormatter } from 'vs/editor/contrib/colorPicker/common/colorFormatter';
 import { Color, RGBA } from 'vs/base/common/color';
-import * as lifecycle from 'vs/base/common/lifecycle';
+import { IDisposable, empty as EmptyDisposable, dispose, combinedDisposable } from 'vs/base/common/lifecycle';
 const $ = dom.$;
 
 class ColorHover {
@@ -182,7 +182,9 @@ export class ModesContentHoverWidget extends ContentHoverWidget {
 	private _modeService: IModeService;
 	private _shouldFocus: boolean;
 	private _colorPicker: ColorPickerWidget;
-	private toDispose: lifecycle.IDisposable[];
+
+	private renderDisposable: IDisposable = EmptyDisposable;
+	private toDispose: IDisposable[];
 
 	constructor(editor: ICodeEditor, openerService: IOpenerService, modeService: IModeService) {
 		super(ModesContentHoverWidget.ID, editor);
@@ -212,11 +214,10 @@ export class ModesContentHoverWidget extends ContentHoverWidget {
 	}
 
 	dispose(): void {
+		this.renderDisposable.dispose();
+		this.renderDisposable = EmptyDisposable;
 		this._hoverOperation.cancel();
-		if (this._colorPicker) {
-			this._colorPicker.dispose();
-		}
-		this.toDispose = lifecycle.dispose(this.toDispose);
+		this.toDispose = dispose(this.toDispose);
 		super.dispose();
 	}
 
@@ -280,10 +281,9 @@ export class ModesContentHoverWidget extends ContentHoverWidget {
 		this._isChangingDecorations = true;
 		this._highlightDecorations = this._editor.deltaDecorations(this._highlightDecorations, []);
 		this._isChangingDecorations = false;
+		this.renderDisposable.dispose();
+		this.renderDisposable = EmptyDisposable;
 		this._colorPicker = null;
-		if (this._colorPicker) {
-			this._colorPicker.dispose();
-		}
 	}
 
 	isColorPickerVisible(): boolean {
@@ -304,6 +304,8 @@ export class ModesContentHoverWidget extends ContentHoverWidget {
 	}
 
 	private _renderMessages(renderRange: Range, messages: HoverPart[]): void {
+		this.renderDisposable.dispose();
+		this._colorPicker = null;
 
 		// update column from which to show
 		var renderColumn = Number.MAX_VALUE,
@@ -348,12 +350,26 @@ export class ModesContentHoverWidget extends ContentHoverWidget {
 				const { red, green, blue, alpha } = msg.color;
 				const rgba = new RGBA(red * 255, green * 255, blue * 255, alpha * 255);
 				const color = new Color(rgba);
-				const model = new ColorPickerModel(color, formatter, availableFormatters, this._editor.getModel(), msg.range);
+				const model = new ColorPickerModel(color, formatter, availableFormatters);
 				const widget = this._register(new ColorPickerWidget(fragment, model, this._editor.getConfiguration().pixelRatio));
 
+				const editorModel = this._editor.getModel();
+				let range = new Range(msg.range.startLineNumber, msg.range.startColumn, msg.range.endLineNumber, msg.range.endColumn);
+
+				const updateEditorModel = () => {
+					const text = model.formatter.formatColor(model.color);
+					editorModel.pushEditOperations([], [{ identifier: null, range, text, forceMoveMarkers: false }], () => []);
+					range = range.setEndPosition(range.endLineNumber, range.startColumn + text.length);
+				};
+
+				const colorListener = model.onDidChangeColor(updateEditorModel);
+				const formatterListener = model.onDidChangeFormatter(updateEditorModel);
+
 				// TODO@Joao woot
 				model.widget = widget;
 				this._colorPicker = widget;
+
+				this.renderDisposable = combinedDisposable([colorListener, formatterListener, widget]);
 			}
 		});