Am 14/03/2023 um 14:14 schrieb Leo Nunner:
> Add a separate tab for the storage edit/create panels to set the
> recently introduced "content-dirs" property which overrides the
> default directory locations. Analogous to the API implementation,
> the tab was added for Directory, CIFS and NFS storages.
> 
> Signed-off-by: Leo Nunner <l.nun...@proxmox.com>
> ---
>  www/manager6/storage/CIFSEdit.js |  8 +++
>  www/manager6/storage/DirEdit.js  | 85 ++++++++++++++++++++++++++++++++
>  www/manager6/storage/NFSEdit.js  |  8 +++
>  3 files changed, 101 insertions(+)
> 

> +Ext.define('PVE.panel.ContentDirsPanel', {
> +    extend: 'Proxmox.panel.InputPanel',

this shouldn't be squished into DirEdit, but rather live in its own file - e.g. 
in
panel/

> +    xtype: 'pveContentDirsTab',
> +
> +    onGetValues: function(form) {

while form isn't wrong, it has IMO merits to keep the more widely used
`values` as parameter name here.

> +     let str = PVE.Parser.printPropertyString(form);
> +     let values = { "content-dirs": str };
> +     return values;

could be as short as:

onGetValues: values => ({ "content-dirs", 
PVE.Parser.printPropertyString(values) }),

> +    },
> +
> +    onSetValues: function(values) {
> +     return values?.["content-dirs"];
> +    },

could be:

onSetValues: values => values?.["content-dirs"],

without loosing really any readability

> +
> +    initComponent: function() {
> +     let me = this;
> +
> +     me.column1 = [
> +         {
> +             xtype: 'textfield',
> +             name: 'images',
> +             fieldLabel: gettext('Disk image'),

We normally use Title Case for field labels.

> +             allowBlank: true,
> +             emptyText: "images",

It would UX do good if you define a vtype (see Toolkit.js in manager or widget 
toolkit)
for the regex used by the backend to help users catching bogus entries early.

Above two (casing & vtype) applies for the remaining fields here.


Note also that having things like disks or the like editable for existing 
storages
can break things fast. I'd maybe be open for having snippets, ISOs and CT 
templates
editable, and even then show a warning hint that existing guest might be 
affected by
the change (i.e., not start anymore). FYI: pmxDisplayEditField is designed for 
such
mixed create/edit -> editable/display-only use-cases.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to