On 3/8/23 10:27, Dominik Csapak wrote:
nice, works really good now with the different interactions
(button/container/changing store/etc)

some comments inline (mostly minor stuff though)

On 3/3/23 16:59, Aaron Lauterer wrote:
[...]
diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js
index b223ab35..9de89df5 100644
--- a/www/manager6/ceph/Status.js
+++ b/www/manager6/ceph/Status.js
@@ -76,6 +76,66 @@ Ext.define('PVE.node.CephStatus', {
              trackRemoved: false,
              data: [],
              },
+            generateTooltipText: (text) => {
+            text = text?.trimStart();

above you indicate that text might be undefined (with the 'text?.'),
but below you don't to any checks again

iff text can really be undefined, i'd do
text = text?.trimStart() ?? '';

otherwise just drop the '?'


hmm yeah, AFAICT there are already checks in place before we even call generateTooltipText -> dropping the '?'


+            if (text.length > 500) {
+                text = `${text.substring(0, 500)}…`;
+            }
+            return text.replaceAll('\n', '<br>');
+            },
+            updateTooltip: (view, isLeave) => {
+            if (!view.tooltip) {

i was rather confused at first what 'view' actually is, but
i found out it's the table view of the grid

couldn't you use always the grid ('this') instead ?
or is there some instance where 'this' isn't the grid?

i think that would make the code a bit more readable, since
you don't have to pass the view around

as discussed off-list: arrow functions do not set 'this' in the same context -> switch to regular 'updateTooltip: function(..) {' instead of arrow functions and in other places too.


[...]


_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to