Re: [pve-devel] [PATCH manager v5 05/11] ui: add form/TagColorGrid

[Matthias Heiserer <m.heiserer@proxmox.com>]

On 08.04.2022 09:45, Dominik Csapak wrote:
8<
> +
> +    setValue: function(value) {
> +	let me = this;
> +	me.setColor(value);
> +	if (value && value.length === 6) {
> +	    me.picker.value = value[0] !== '#' ? `#${value}` : value;
> +	}
> +    },
> +
> +    setColor: function(value) {
> +	let me = this;
> +	let oldValue = me.realvalue;
> +	me.realvalue = value;
> +	let color = value.length === 6 ? `#${value}` : undefined;
> +	me.down('#picker').setStyle('background-color', color);Not a big fan of this initially being white. Makes it somewhat hard to 
discover.

8<
> +
> +Ext.define('PVE.form.TagColorGrid', {
> +    extend: 'Ext.grid.Panel',
> +    alias: 'widget.pveTagColorGrid',
> +
> +    mixins: [
> +	'Ext.form.field.Field',
> +    ],
> +

I don't like how the selection in the grid works. I.e. one has to click 
the sliver of whitespace between the columns to select a row, while 
writing doesn't change the selected column.

8<---
> +	},
> +    ],
> +
> +    columns: [
> +	{
> +	    header: 'Tag',
> +	    dataIndex: 'tag',
> +	    xtype: 'widgetcolumn',
> +	    onWidgetAttach: function(col, widget, rec) {
> +		widget.getStore().setData(PVE.Utils.tagList.map(v => ({ tag: v })));
> +	    },
> +	    widget: {
> +		xtype: 'combobox',
> +		isFormField: false,
> +		maskRe: /[a-zA-Z0-9_.-]/,
As it gets used several times in different locations (form/Tag), you 
could consider storing it in a fixed place.

8<---