Re: [PATCH manager v2] ui: ha: add disarm/re-arm button

["Dominik Rusovac" <d.rusovac@proxmox.com>]

thx for the comments! I will send a v3

On Wed Apr 15, 2026 at 2:32 PM CEST, Dominik Csapak wrote:
> what i miss with using these is some feedback when i activated them.
>
> This is probably more due to the backend decision, but
> when clicking arm/disarm, there is nothing happening in the gui at
> first, no spinning icon, no change in state..

regarding feedback after activating either of them: one can see the
Status changing in the Status panel, which exactly traces what's going
on for every node, in particular it reveals the armed-state in the
Status of fencing

if that's not enough feedback, I'd look into other possible solutions in
more detail

>
> I think there are a few possible solutions on this, but
> i'm not super deep in the ha stack, so sorry in advance if
> some can't work:
> * use a worker that waits for the change of the ha state, e.g.
>    via polling. this could directly be shown in the gui
> * mark the requested state immediately somewhere and return
>    it with the overall ha state, so we  see in the gui what is happening
> * 'fake' the progress until we see a status change
>    (i don't really like this, since it's prone to errors when e.g.
>     one admin disarms, the other arms again, but the gui does not
>     get an updated state in the meantime)
>
>
> other comments inline
>
> On 4/15/26 8:40 AM, 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 button
>> to arm HA is disabled as long as HA is not disarmed.
>> 
>> The icons ('unlink' and 'link') are chosen to emphasize that "Disarm HA"
>> and "Arm HA" are complements. There may be more suitable pairs of icons
>> though.
>> 
>> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
>> ---
>> changes since v1:
>> * use camel case for function names
>> * choose clearer parameter: 'comp' -> 'menuItem'
>> * choose clearer function names:
>>    - 'disarm' -> 'handleDisarmButton'
>>    - 'rearm'  -> 'handleArmButton'
>> * deduplicate translation keys
>> * use same translation keys as for menu items
>> * remove obsolete 'params'
>> * change button text "Re-arm HA" to "Arm HA"
>> * remove extra new-line
>> 
>>   www/manager6/ha/Status.js     | 104 ++++++++++++++++++++++++++++++++++
>>   www/manager6/ha/StatusView.js |   8 +++
>>   2 files changed, 112 insertions(+)
>> 

[snip]

>> +    controller: {
>> +        xclass: 'Ext.app.ViewController',
>> +
>> +        checkHaStatus: function (isDisarmed) {
>> +            let vm = this.getViewModel();
>> +            vm.set('haDisarmed', isDisarmed);
>> +        },
>> +
>
> this method does not 'check' it 'sets' the HA status?
>
> IMHO a better name would be:
>
> setHaStatus
> or similar
>
>
> also, this is only used once, so it's probably better
> to just call the vm.set inline
>

thx, I will inline it

[snip]

>> +
>> +    dockedItems: [
>> +        {
>> +            xtype: 'toolbar',
>> +            dock: 'top',
>> +            items: [
>
> it should be able to shorten that to just using 'tbar'
>
> tbar: [
>      { ... }, // button 1
>      { ... }, // button 2
> ],
>

indeed, thx, will use 'tbar' instead

>> +                {
>> +                    text: gettext('Disarm HA'),
>> +                    iconCls: 'fa fa-unlink',
>> +                    bind: {
>> +                        disabled: '{haDisarmed}',
>> +                    },
>> +                    menu: [
>> +                        {
>> +                            text: gettext('Freeze'),
>> +                            iconCls: 'fa fa-snowflake-o',
>> +                            mode: 'freeze',
>> +                            handler: 'handleDisarmButton',
>> +                        },
>> +                        {
>> +                            text: gettext('Ignore'),
>> +                            iconCls: 'fa fa-eye-slash',
>> +                            mode: 'ignore',
>> +                            handler: 'handleDisarmButton',
>> +                        },
>> +                    ],
>> +                },
>> +                {
>> +                    text: gettext('Arm HA'),
>> +                    iconCls: 'fa fa-link',
>> +                    bind: {
>> +                        disabled: '{!haDisarmed}',
>> +                    },
>> +                    handler: 'handleArmButton',
>> +                },
>> +            ],
>
> i'm not totally against having two buttons here, but since
> there is always only one or the other active, wouldn't
> it make more sense to hide the button that
> can't do anything? (though i admit we do most often show the
> unusable buttons across our gui, so it's fine too)
>

tbh, I just tried to adhere to what I found in other places of the gui

if hiding the unusable button is more desirable, I can go for that
option instead

> At least i would probably switch the position of these,
> since the more  (for the lack of a better word) 'affirming' action
> is usually at the beginning (like 'add' or 'start') and the other
> options comes later (like 'remove' or 'shutdown')
>
> so
>
> | Arm | Disarm |
>
> reads more logical to me than
>
> | Disarm | Arm |
>

in retrospect, I think my reasoning behind this positioning was rather
about the more 'obvious' or 'natural' action, that is: HA is
from the get-go (and also usually) armed and can only be (re-)armed,
once it's been disarmed - just like stopping something is only possible
once this very something's started 

I see your reasoning though, and if we do not go for hiding either of
the buttons, I do not mind changing the positioning 

>> +        },
>> +    ],
>> +

[snip]

>> +            me.rstore.on('load', function () {
>> +                let fencing = store.findRecord('type', 'fencing');
>> +                let disarmed = fencing && fencing.get('armed-state') === 'disarmed';
>> +
>> +                me.fireEvent('hastatuschange', disarmed);
>
> just a remark, not even a nit:
>
> the usual convention in extjs is that the first parameter of an event is
> the component itself, so it would usually be
>
> me.fireEvent('foo', me, <remaining arguments>);
>
> we (and extjs itself) don't always adhere to that, so it's fine
> here, but wanted to note it anyway
>

thx for this informative remark

[snip]