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

Dominik Rusovac
2026-04-15T13:32:42Z

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]