[pve-devel] [PATCH widget-toolkit 1/2] add advanced options to the input panel

[Thomas Lamprecht]

I did not tested much yet (will do soon), so purely code/idea review for 
now.

Am 04/05/2018 um 04:03 PM schrieb Dominik Csapak:
> with this, you can now put items in the
> advancedColumn1/2/B and show/hide it with
> setAdvancedVisible
> 

Hmm, would it be much nicer if we could just add a config (e.g.,
'advanced') to a component?

E.g.,

[...],
{
     xtype: 'proxmoxtextfield',
     name: 'complicate_setting',
     [...],
     advanced: true
}

The on a showAdvanced change all does (and their childs?) components
will be disabled and hidden, or enabled and shown, respectively.

This would make the change of transforming and existing item to advanved
very small.
Also you duplicate the column{1,2,B} configs for panels, components get
sprincled all over the place, makes it harder to read the code and find
stuff, IMO.

> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>   panel/InputPanel.js | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 79 insertions(+)
> 
> diff --git a/panel/InputPanel.js b/panel/InputPanel.js
> index e3a2520..4bc5777 100644
> --- a/panel/InputPanel.js
> +++ b/panel/InputPanel.js
> @@ -20,6 +20,13 @@ Ext.define('Proxmox.panel.InputPanel', {
>       // setting this will display a help button in our parent panel
>       onlineHelp: undefined,
>   
> +    // will be set if the inputpanel has advanced items

nit: unnecessary comment, you could name the property 'showAdvancedItems'
to make it really clear, if you think 'showAdvanced' is to ambiguos.
We have a few such comments, mirroring the ExtJS style - but there it
makes more sense as they generate the framework docs directly out of the
code...

> +    hasAdvanced: false,
> +
> +    // if the panel has advanced items,
> +    // this will determine if they are shown by default
> +    showAdvanced: false,
> +
>       // overwrite this to modify submit data
>       onGetValues: function(values) {
>   	return values;
> @@ -43,6 +50,14 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	return me.onGetValues(values);
>       },
>   
> +    setAdvancedVisible: function(visible) {
> +	var me = this;
> +	var advItems = me.getComponent('advancedContainer');
> +	if (advItems) {
> +	    advItems.setVisible(visible);
> +	}
> +    },
> +
>       setValues: function(values) {
>   	var me = this;
>   
> @@ -138,6 +153,70 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	    throw "unsupported config";
>   	}
>   
> +	var advItems;
> +	if (me.advancedItems) {
> +	    advItems = [
> +		{
> +		    columnWidth: 1,
> +		    layout: 'anchor',
> +		    items: me.advancedItems
> +		}
> +	    ];
> +	    me.advancedItems = undefined;
> +	} else if (me.advancedColumn1) {
> +	    advItems = [
> +		{
> +		    columnWidth: 0.5,
> +		    padding: '0 10 0 0',
> +		    layout: 'anchor',
> +		    items: me.advancedColumn1
> +		},
> +		{
> +		    columnWidth: 0.5,
> +		    padding: '0 0 0 10',
> +		    layout: 'anchor',
> +		    items: me.advancedColumn2 || [] // allow empty column
> +		}
> +	    ];
> +
> +	    me.advancedColumn1 = undefined;
> +	    me.advancedColumn2 = undefined;
> +
> +	    if (me.advancedColumnB) {
> +		advItems.push({
> +		    columnWidth: 1,
> +		    padding: '10 0 0 0',
> +		    layout: 'anchor',
> +		    items: me.advancedColumnB
> +		});
> +		me.advancedColumnB = undefined;
> +	    }
> +	}
> +
> +	if (advItems) {
> +	    me.hasAdvanced = true;
> +	    advItems.unshift({
> +		columnWidth: 1,
> +		xtype: 'box',
> +		hidden: false,
> +		border: true,
> +		autoEl: {
> +		    tag: 'hr'
> +		}
> +	    });
> +	    items.push({
> +		columnWidth: 1,
> +		xtype: 'container',
> +		itemId: 'advancedContainer',
> +		hidden: !me.showAdvanced,
> +		layout: 'column',
> +		defaults: {
> +		    border: false
> +		},
> +		items: advItems
> +	    });
> +	}
> +
>   	if (me.useFieldContainer) {
>   	    Ext.apply(me, {
>   		layout: 'fit',
>