[pve-devel] [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

[pve-devel] [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

[Matthias Heiserer]

The same code is used once in widget toolkit and twice in PVE already,
so it makes sense to add it as a separate button.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
changes from v1:
move into separate class
rename vars to something a little bit nicer
move comment above function
reorder some statements

Note: Because it's now guaranteed that the function is called at render time,
I think we can leave out the extra logic to test which text is currently set.


 src/button/AltText.js | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 src/button/AltText.js

diff --git a/src/button/AltText.js b/src/button/AltText.js
new file mode 100644
index 0000000..e74d042
--- /dev/null
+++ b/src/button/AltText.js
@@ -0,0 +1,22 @@
+Ext.define('Proxmox.button.AltText', {
+    extend: 'Proxmox.button.Button',
+    xtype: 'proxmoxAltTextButton',
+
+    defaultText: "",
+    altText: "",
+
+    listeners: {
+	// HACK: calculate the max button width on first render to avoid toolbar glitches
+	render: function(button) {
+	    let me = this;
+
+	    button.setText(me.altText);
+	    let altWidth = button.getSize().width;
+
+	    button.setText(me.defaultText);
+	    let defaultWidth = button.getSize().width;
+
+	    button.setWidth(defaultWidth > altWidth ? defaultWidth : altWidth);
+	},
+    },
+});
-- 
2.30.2

[pve-devel] [PATCH v2 widget-toolkit 2/3] Makefile: add AltText

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 src/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Makefile b/src/Makefile
index abafc2c..76dc012 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -45,6 +45,7 @@ JSSRC=					\
 	form/ACME.js			\
 	form/UserSelector.js		\
 	button/Button.js		\
+	button/AltText.js		\
 	button/HelpButton.js		\
 	grid/ObjectGrid.js		\
 	grid/PendingObjectGrid.js	\
-- 
2.30.2

[pve-devel] [PATCH v2 widget-toolkit 3/3] apt repos: use new AltText button

[Matthias Heiserer]

text is not needed with the AltText button.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 src/node/APTRepositories.js | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 09ed0be..ce8f718 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -193,8 +193,7 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	},
 	'-',
 	{
-	    xtype: 'proxmoxButton',
-	    text: gettext('Enable'),
+	    xtype: 'proxmoxAltTextButton',
 	    defaultText: gettext('Enable'),
 	    altText: gettext('Disable'),
 	    name: 'repoEnable',
@@ -229,18 +228,6 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 		    },
 		});
 	    },
-	    listeners: {
-		render: function(btn) {
-		    // HACK: calculate the max button width on first render to avoid toolbar glitches
-		    let defSize = btn.getSize().width;
-
-		    btn.setText(btn.altText);
-		    let altSize = btn.getSize().width;
-
-		    btn.setText(btn.defaultText);
-		    btn.setSize({ width: altSize > defSize ? altSize : defSize });
-		},
-	    },
 	},
     ],
 
-- 
2.30.2

[pve-devel] [PATCH v2 manager] ui: tree-wide: use new AltText button

[Matthias Heiserer]

text is not needed with the AltText button.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/lxc/Resources.js     | 18 +-----------------
 www/manager6/qemu/HardwareView.js | 18 +-----------------
 2 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 15ee3c67..96dc43a1 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -189,8 +189,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    handler: run_resize,
 	});
 
-	var remove_btn = new Proxmox.button.Button({
-	    text: gettext('Remove'),
+	var remove_btn = new Proxmox.button.AltText({
 	    defaultText: gettext('Remove'),
 	    altText: gettext('Detach'),
 	    selModel: me.selModel,
@@ -210,21 +209,6 @@ Ext.define('PVE.lxc.RessourceView', {
 		return msg;
 	    },
 	    handler: run_remove,
-	    listeners: {
-		render: function(btn) {
-		    // hack: calculate the max button width on first display to prevent the whole
-		    // toolbar to move when we switch between the "Remove" and "Detach" labels
-		    let def = btn.getSize().width;
-
-		    btn.setText(btn.altText);
-		    let alt = btn.getSize().width;
-
-		    btn.setText(btn.defaultText);
-
-		    let optimal = alt > def ? alt : def;
-		    btn.setSize({ width: optimal });
-		},
-	    },
 	});
 
 	var move_btn = new Proxmox.button.Button({
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 6cea4287..a95da1dc 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -438,8 +438,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    handler: run_move,
 	});
 
-	var remove_btn = new Proxmox.button.Button({
-	    text: gettext('Remove'),
+	var remove_btn = new Proxmox.button.AltText({
 	    defaultText: gettext('Remove'),
 	    altText: gettext('Detach'),
 	    selModel: sm,
@@ -485,21 +484,6 @@ Ext.define('PVE.qemu.HardwareView', {
 		    },
 		});
 	    },
-	    listeners: {
-		render: function(btn) {
-		    // hack: calculate the max button width on first display to prevent the whole
-		    // toolbar to move when we switch between the "Remove" and "Detach" labels
-		    var def = btn.getSize().width;
-
-		    btn.setText(btn.altText);
-		    var alt = btn.getSize().width;
-
-		    btn.setText(btn.defaultText);
-
-		    var optimal = alt > def ? alt : def;
-		    btn.setSize({ width: optimal });
-		},
-	    },
 	});
 
 	var revert_btn = new PVE.button.PendingRevert({
-- 
2.30.2

Re: [pve-devel] [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

[Dominik Csapak]

patches LGTM, 2/3 could be squashed into 1/3 so that we have
the new code in the same commit as the Makefile change,
or what do you think @thomas? do you prefer those things
to be split into 2 patches?

on an additional note, we could even add more functionality to the
button, namely a simple 'setTextVariant' (or similar) so that the
caller can easily change between the normal and alt text.
Also a getter (like 'isAltText' or 'isNormalText') would be nice

for now the users of this button always access the 'altText' and 'text'
property directly which doesn't seem that nice if we already have
a special class for it

but this could be done later too, so no hard feelings

On 3/28/22 15:07, Matthias Heiserer wrote:
> The same code is used once in widget toolkit and twice in PVE already,
> so it makes sense to add it as a separate button.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> changes from v1:
> move into separate class
> rename vars to something a little bit nicer
> move comment above function
> reorder some statements
> 
> Note: Because it's now guaranteed that the function is called at render time,
> I think we can leave out the extra logic to test which text is currently set.
> 
> 
>   src/button/AltText.js | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>   create mode 100644 src/button/AltText.js
> 
> diff --git a/src/button/AltText.js b/src/button/AltText.js
> new file mode 100644
> index 0000000..e74d042
> --- /dev/null
> +++ b/src/button/AltText.js
> @@ -0,0 +1,22 @@
> +Ext.define('Proxmox.button.AltText', {
> +    extend: 'Proxmox.button.Button',
> +    xtype: 'proxmoxAltTextButton',
> +
> +    defaultText: "",
> +    altText: "",
> +
> +    listeners: {
> +	// HACK: calculate the max button width on first render to avoid toolbar glitches
> +	render: function(button) {
> +	    let me = this;
> +
> +	    button.setText(me.altText);
> +	    let altWidth = button.getSize().width;
> +
> +	    button.setText(me.defaultText);
> +	    let defaultWidth = button.getSize().width;
> +
> +	    button.setWidth(defaultWidth > altWidth ? defaultWidth : altWidth);
> +	},
> +    },
> +});

Re: [pve-devel] [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

[Dominik Csapak]

ah i forgot:

consider the series:

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>

[pve-devel] applied: [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

[Thomas Lamprecht]

On 28.03.22 15:07, Matthias Heiserer wrote:
> The same code is used once in widget toolkit and twice in PVE already,
> so it makes sense to add it as a separate button.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> changes from v1:
> move into separate class
> rename vars to something a little bit nicer
> move comment above function
> reorder some statements
> 
> Note: Because it's now guaranteed that the function is called at render time,
> I think we can leave out the extra logic to test which text is currently set.
> 
> 
>  src/button/AltText.js | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 src/button/AltText.js
> 
>

applied with 2/3 squashed in, thanks!