From: "Daniel Kral" <d.kral@proxmox.com>
To: "Dominik Rusovac" <d.rusovac@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH manager] ui: ha: add disarm/re-arm button
Date: Tue, 14 Apr 2026 15:30:29 +0200 [thread overview]
Message-ID: <DHSWXKSN8XXC.3HHSOBZ41QXXS@proxmox.com> (raw)
In-Reply-To: <20260414115156.188522-1-d.rusovac@proxmox.com>
Nice work, looks great!
I added some inline comments, but functionality-wise this works as
expected already.
On Tue Apr 14, 2026 at 1:51 PM CEST, Dominik Rusovac wrote:
> The button to disarm HA in either of the resource modes ('freeze' or
> 'ignore') is disabled as long as HA is disarmed. Analogously, the
> re-arming button is disabled as long as HA is not disarmed.
>
> The icons ('unlink' and 'link') are chosen to emphasize that
> "Disarm HA" and "Re-arm HA" are complements. There may be more
> suitable pairs of icons though.
>
> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
> ---
> www/manager6/ha/Status.js | 106 ++++++++++++++++++++++++++++++++++
> www/manager6/ha/StatusView.js | 8 +++
> 2 files changed, 114 insertions(+)
>
> diff --git a/www/manager6/ha/Status.js b/www/manager6/ha/Status.js
> index b0b0feb9..d955452d 100644
> --- a/www/manager6/ha/Status.js
> +++ b/www/manager6/ha/Status.js
> @@ -8,6 +8,107 @@ Ext.define('PVE.ha.Status', {
> align: 'stretch',
> },
>
> + viewModel: {
> + data: {
> + haDisarmed: false,
> + },
> + },
> +
> + controller: {
> + xclass: 'Ext.app.ViewController',
> +
> + check_ha_status: function (isDisarmed) {
new method names should use cameCase according to our JS Style Guide [0]
> + let vm = this.getViewModel();
> + vm.set('haDisarmed', isDisarmed);
> + },
> +
> + disarm: function (comp) {
nit: maybe s/comp/menuItem/ would make it a little clearer where the
information is coming from?
nit: maybe s/disarm/handleDisarmButton/ would also make it clearer where
it is supposed to be used (instead of a generic method here)
> + let me = this;
> + let view = me.getView();
> + let mode = comp.mode;
> +
> + Ext.Msg.confirm(
> + gettext('Confirm'),
> + mode === 'freeze'
> + ? gettext("Are you sure you want to disarm HA with resource mode 'freeze'?")
> + : gettext("Are you sure you want to disarm HA with resource mode 'ignore'?"),
nit: the translation keys could be deduplicated with something like
Ext.Msg.confirm(
gettext('Confirm'),
Ext.String.format(
gettext("Are you sure you want to disarm HA with resource mode '{0}'?"),
mode
)
...
);
Feel free to restructure the code snippet if you consider using it.
I also removed the mode === 'freeze' condition with just emplacing the
content of `mode`, but it would be nicer to use the same translation
keys as for the menu items (i.e., gettext('Freeze') and
gettext('Ignore')).
> + function (btn) {
> + if (btn !== 'yes') {
> + return;
> + }
> + Proxmox.Utils.API2Request({
> + url: '/cluster/ha/status/disarm-ha',
> + params: { 'resource-mode': mode },
> + method: 'POST',
> + waitMsgTarget: view,
> + failure: (response) => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> + });
> + },
> + );
> + },
> +
> + rearm: function () {
nit: maybe s/rearm/handleRearmButton/ here as well?
> + let me = this;
> + let view = me.getView();
> +
> + Ext.Msg.confirm(
> + gettext('Confirm'),
> + gettext('Are you sure you want to re-arm HA?'),
> + function (btn) {
> + if (btn !== 'yes') {
> + return;
> + }
> + Proxmox.Utils.API2Request({
> + url: '/cluster/ha/status/arm-ha',
> + params: {},
nit: the params is not needed here
> + method: 'POST',
> + waitMsgTarget: view,
> + failure: (response) => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> + });
> + },
> + );
> + },
> + },
> +
> + dockedItems: [
> + {
> + xtype: 'toolbar',
> + dock: 'top',
> + items: [
> + {
> + text: gettext('Disarm HA'),
> + iconCls: 'fa fa-unlink',
> + bind: {
> + disabled: '{haDisarmed}',
> + },
> + menu: [
> + {
> + text: gettext('Freeze'),
> + iconCls: 'fa fa-snowflake-o',
> + mode: 'freeze',
> + handler: 'disarm',
> + },
> + {
> + text: gettext('Ignore'),
> + iconCls: 'fa fa-eye-slash',
> + mode: 'ignore',
> + handler: 'disarm',
> + },
> + ],
> + },
> + {
> + text: gettext('Re-arm HA'),
nit: wondering whether this should just be called 'Arm HA', but no hard
feelings about this. If we go for that, should be changed for the
other instances as well.
> + iconCls: 'fa fa-link',
> + bind: {
> + disabled: '{!haDisarmed}',
> + },
> + handler: 'rearm',
> + },
> + ],
> + },
> + ],
> +
> +
extra new-line that is removed with a `make -C www/manager6 tidy`
> initComponent: function () {
> var me = this;
>
> @@ -30,6 +131,11 @@ Ext.define('PVE.ha.Status', {
> border: 0,
> collapsible: true,
> padding: '0 0 20 0',
> + listeners: {
> + hastatuschange: function (isDisarmed) {
> + me.getController().check_ha_status(isDisarmed);
> + },
> + },
> },
> {
> xtype: 'pveHAResourcesView',
[0] https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing
next prev parent reply other threads:[~2026-04-14 13:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-14 11:51 Dominik Rusovac
2026-04-14 13:30 ` Daniel Kral [this message]
2026-04-14 13:49 ` Dominik Rusovac
2026-04-15 6:44 ` Dominik Rusovac
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=DHSWXKSN8XXC.3HHSOBZ41QXXS@proxmox.com \
--to=d.kral@proxmox.com \
--cc=d.rusovac@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