[pve-devel] [PATCH v2 manager 1/2] gui/cluster: add CorosyncLinkEdit component to support up to 8 links

[Thomas Lamprecht]

On 3/23/20 10:50 AM, Stefan Reiter wrote:
> On 19/03/2020 15:06, Dominik Csapak wrote:
>> overall looks good, and most works, comments inline
>>
>> the only 'real' issue i found is with ipv6 networks
>> (they do not get auto added to the links)
>>
> 
> Thanks for the review, I'll send a fixed v3 soon!
> 
>> On 3/17/20 12:11 PM, Stefan Reiter wrote
>>> -        ring1Possible: false,
>>>           ip: '',
>>>           clusterName: ''
>>>           };
>>> -        var totem = {};
>>>           if (!(joinInfo && joinInfo.totem)) {
>>>           field.valid = false;
>>> +        linkEditor.setLinks([]);
>>> +        linkEditor.setInfoText();
>>>           } else {
>>> -        var ring0Needed = false;
>>> -        if (joinInfo.ring_addr !== undefined) {
>>> -            ring0Needed = joinInfo.ring_addr[0] !== joinInfo.ipAddress;
>>> +        var links = [];
>>> +        var interfaces = joinInfo.totem['interface'];
>>
>> nit: i'd rather use 'let' whenever possible for new code
>>
> 
> I used it here to not mix-and-match var and let in existing functions. Is this the intent, or should I also change other 'var's over to let then?

let for new, certain mixing is OK (we won't move forward else)

> 
>>>       columnB: [
>>>           {
>>>           xtype: 'textfield',
>>>           fieldLabel: gettext('Fingerprint'),
>>> +        style: 'margin-top: -10px',
>>
>> same as for 'padding-top'
>> also, why '-10px' ?
>>
>> also, the whole layout seems kinda wonky (at least with
>> the language set to german), the password field
>> is higher up that the address field...
>>
> 
> Hadn't tested in other languages... Probably should have ;)
> 
> The reason for the '-10px' was to avoid exactly that issue in the English layout, where without it some text fields would mysteriously be misaligned. I'll try and figure out how to fix this correctly and for all localizations.
> 

thanks, FYI: Dominik is on vacation this week, so I'll try to give a v3
a faster look.