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?

      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.

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

Reply via email to