public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Cc: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-manager v4] ui: fix #6209: create snapshots and backups from context menu
Date: Wed, 08 Oct 2025 11:51:53 +0200	[thread overview]
Message-ID: <DDCUJSMMZK8O.ULJY3RQ3RAWV@proxmox.com> (raw)
In-Reply-To: <20251007121352.102236-1-n.frey@proxmox.com>

On Tue Oct 7, 2025 at 2:13 PM CEST, Nicolas Frey wrote:
> The original feature request only specified the ability to take
> snapshots. I added the manual backup shortcut as well, since
> navigating through the menu for this action felt unnecessarily
> cumbersome when performing just a quick backup.
>
> Changes since v3:
> * replace anonymous functions with arrow style functions
> * reverted approach from v2 after some experimentation, simulating
>     latency using chrome dev tools + traffic control
> * uses the same approach as in v1/v2 to match behaviour seen in the
>     Snapshot tab
>
> Changes since v2:
> * rebased
> * use autoShow on snapshot and backup windows
> * improve UX and account for slow connections by only making API
>   calls for snapshot feature on button click
>
> changes since v1, thanks @Fiona Ebner
> * clarified commit message
> * addressed several issues in the added menu options, namely:
>   * correctly handle disabled state with async
>   * fixed negation logic and cleaned up minor nits
>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6209
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
>  www/manager6/lxc/CmdMenu.js  | 49 ++++++++++++++++++++++++++++++++++++
>  www/manager6/qemu/CmdMenu.js | 49 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
>
> diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
> index cd60c967..a7940e04 100644
> --- a/www/manager6/lxc/CmdMenu.js
> +++ b/www/manager6/lxc/CmdMenu.js
> @@ -134,6 +134,40 @@ Ext.define('PVE.lxc.CmdMenu', {
>                  },
>              },
>              { xtype: 'menuseparator' },
> +            {
> +                text: gettext('Take Snapshot'),
> +                iconCls: 'fa fa-fw fa-history',
> +                itemId: 'takeSnapshotBtn',
> +                disabled: true,
> +                handler: () => {
> +                    Ext.create('PVE.window.Snapshot', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmname: info.name,
> +                        viewonly: false,
> +                        type: info.type,
> +                        isCreate: true,
> +                        submitText: gettext('Take Snapshot'),
> +                        autoShow: true,
> +                        running: running,
> +                    });
> +                },
> +            },
> +            {
> +                text: gettext('Backup now'),
> +                iconCls: 'fa fa-fw fa-floppy-o',
> +                disabled: !caps.vms['VM.Backup'],
> +                handler: () => {
> +                    Ext.create('PVE.window.Backup', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmtype: info.type,
> +                        vmname: info.name,
> +                        autoShow: true,
> +                    });
> +                },
> +            },
> +            { xtype: 'menuseparator' },
>              {
>                  text: gettext('Console'),
>                  iconCls: 'fa fa-fw fa-terminal',
> @@ -149,5 +183,20 @@ Ext.define('PVE.lxc.CmdMenu', {
>          ];
>
>          me.callParent();
> +
> +        if (caps.vms['VM.Snapshot']) {
> +            Proxmox.Utils.API2Request({
> +                url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`,
> +                params: { feature: 'snapshot' },
> +                method: 'GET',
> +                success: (response) => {
> +                    let hasFeature = response.result.data.hasFeature;
> +                    let btn = me.down('#takeSnapshotBtn');
> +                    if (btn) {
> +                        btn.setDisabled(!hasFeature);
> +                    }
> +                },
> +            });
> +        }
>      },
>  });
> diff --git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js
> index adf64672..df6045bf 100644
> --- a/www/manager6/qemu/CmdMenu.js
> +++ b/www/manager6/qemu/CmdMenu.js
> @@ -169,6 +169,40 @@ Ext.define('PVE.qemu.CmdMenu', {
>                  },
>              },
>              { xtype: 'menuseparator' },
> +            {
> +                text: gettext('Take Snapshot'),
> +                iconCls: 'fa fa-fw fa-history',
> +                itemId: 'takeSnapshotBtn',
> +                disabled: true,
> +                handler: () => {
> +                    Ext.create('PVE.window.Snapshot', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmname: info.name,
> +                        viewonly: false,
> +                        type: info.type,
> +                        isCreate: true,
> +                        submitText: gettext('Take Snapshot'),
> +                        autoShow: true,
> +                        running: running,
> +                    });
> +                },
> +            },
> +            {
> +                text: gettext('Backup now'),
> +                iconCls: 'fa fa-fw fa-floppy-o',
> +                disabled: !caps.vms['VM.Backup'],
> +                handler: () => {
> +                    Ext.create('PVE.window.Backup', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmtype: info.type,
> +                        vmname: info.name,
> +                        autoShow: true,
> +                    });
> +                },
> +            },
> +            { xtype: 'menuseparator' },
>              {
>                  text: gettext('Console'),
>                  iconCls: 'fa fa-fw fa-terminal',
> @@ -194,5 +228,20 @@ Ext.define('PVE.qemu.CmdMenu', {
>          ];
>
>          me.callParent();
> +
> +        if (caps.vms['VM.Snapshot']) {
> +            Proxmox.Utils.API2Request({
> +                url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`,
> +                params: { feature: 'snapshot' },
> +                method: 'GET',
> +                success: (response) => {
> +                    let hasFeature = response.result.data.hasFeature;
> +                    let btn = me.down('#takeSnapshotBtn');
> +                    if (btn) {
> +                        btn.setDisabled(!hasFeature);
> +                    }
> +                },
> +            });
> +        }
>      },
>  });

looks good to me now, so consider this:

Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>


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


      reply	other threads:[~2025-10-08  9:52 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-07 12:13 Nicolas Frey
2025-10-08  9:51 ` Shannon Sterz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DDCUJSMMZK8O.ULJY3RQ3RAWV@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=pve-devel-bounces@lists.proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal