[pve-devel] [PATCH manager v2 0/2] ui: tasks: add button to open task

[pve-devel] [PATCH manager v2 0/2] ui: tasks: add button to open task

[Aaron Lauterer]

the first patch adds a new action column to open the task.

this will hopefully make it clearer for (new) users that a task can be
opened.

The second, opional, patch, adapts the code to the more current way of
opening a new window.

changes:
v1: implement code improvement in patch 1/2

Aaron Lauterer (2):
  ui: tasks: add actioncolumn to open task
  ui: tasks: use autoshow instead of win.show

 www/manager6/dc/Tasks.js | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
2.39.5



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

[pve-devel] [PATCH manager v2 1/2] ui: tasks: add actioncolumn to open task

[Aaron Lauterer]

Many (new) users don't realize that the task view can be used to open
the task details.
By adding a small action column we can have a button to open the task as
well, next to double clicking it.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
Since the record is fetched quite differently than in the regular
handler, and we don't necessarily have a selection on the right row at
this point, I opted to create the window right in the dedicated handler.

The other sensible approach would be to factor out the window opening
from `run_task_viewer` into a new function. But since that function
would just open the window I decided against it.

Not sure if we want the above explanation in the commit msg. If so, feel
free to put it there when applying the patch.

Regarding the icon: I went with the chevron-right as this is what we use
in the UI of the backup server. Alternatively the `fa-eye` might also be
fitting.

change since v1:
* use the `rec` parameter directly instead of fetching if from the grid
store

 www/manager6/dc/Tasks.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/www/manager6/dc/Tasks.js b/www/manager6/dc/Tasks.js
index 2001bf76..4b542762 100644
--- a/www/manager6/dc/Tasks.js
+++ b/www/manager6/dc/Tasks.js
@@ -127,6 +127,25 @@ Ext.define('PVE.dc.Tasks', {
 			return Proxmox.Utils.format_task_status(value);
 		    },
 		},
+		{
+		    xtype: 'actioncolumn',
+		    width: 30,
+		    align: 'center',
+		    tooltip: gettext('Actions'),
+		    items: [
+			{
+			    iconCls: 'fa fa-chevron-right',
+			    tooltip: gettext('Show Task'),
+			    handler: function(_grid, _rowIndex, _colIndex, _item, _e, rec) {
+				    Ext.create('Proxmox.window.TaskViewer', {
+					autoShow: true,
+					upid: rec.data.upid,
+					endtime: rec.data.endtime,
+				    });
+			    },
+			},
+		    ],
+		},
 	    ],
 	    listeners: {
 		itemdblclick: run_task_viewer,
-- 
2.39.5



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

Re: [pve-devel] [PATCH manager v2 1/2] ui: tasks: add actioncolumn to open task

[Thomas Lamprecht]

Am 11.11.24 um 16:50 schrieb Aaron Lauterer:
> Many (new) users don't realize that the task view can be used to open
> the task details.
> By adding a small action column we can have a button to open the task as
> well, next to double clicking it.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> Since the record is fetched quite differently than in the regular
> handler, and we don't necessarily have a selection on the right row at
> this point, I opted to create the window right in the dedicated handler.
> 
> The other sensible approach would be to factor out the window opening
> from `run_task_viewer` into a new function. But since that function
> would just open the window I decided against it.
> 
> Not sure if we want the above explanation in the commit msg. If so, feel
> free to put it there when applying the patch.
> 
> Regarding the icon: I went with the chevron-right as this is what we use
> in the UI of the backup server. Alternatively the `fa-eye` might also be
> fitting.

Looks fine code-wise, but we got some inconsistency with these things:

- PBS uses the same icon but uses "Open Task" as tooltip
- our dedicated Task panel, used for the per-node or per-guest task logs
  is using "View" as button text with fa-window-restore as icon.
  Also, it would be good to think over if it makes sense adding the action
  column there too, albeit I'm not having problems with skipping that for
  now

fa-eye is IMO worse on its own and, more objectively, as that it further
increases the grade of inconsistency.




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

[pve-devel] [PATCH manager v2 2/2] ui: tasks: use autoshow instead of win.show

[Aaron Lauterer]

This is the current approach to create new windows and will align it to
the other handler (action column).

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
no changes since v1

 www/manager6/dc/Tasks.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/dc/Tasks.js b/www/manager6/dc/Tasks.js
index 4b542762..24b2697f 100644
--- a/www/manager6/dc/Tasks.js
+++ b/www/manager6/dc/Tasks.js
@@ -42,11 +42,11 @@ Ext.define('PVE.dc.Tasks', {
 		return;
 	    }
 
-	    var win = Ext.create('Proxmox.window.TaskViewer', {
+	    Ext.create('Proxmox.window.TaskViewer', {
+		autoShow: true,
 		upid: rec.data.upid,
 		endtime: rec.data.endtime,
 	    });
-	    win.show();
 	};
 
 	Ext.apply(me, {
-- 
2.39.5



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

[pve-devel] applied: [PATCH manager v2 2/2] ui: tasks: use autoshow instead of win.show

[Thomas Lamprecht]

Am 11.11.24 um 16:50 schrieb Aaron Lauterer:
> This is the current approach to create new windows and will align it to
> the other handler (action column).
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> no changes since v1
> 
>  www/manager6/dc/Tasks.js | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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

Re: [pve-devel] [PATCH manager v2 0/2] ui: tasks: add button to open task

[Aaron Lauterer]

sent a v3 with ideas and feedback received

https://lore.proxmox.com/pve-devel/20241118104530.91798-1-a.lauterer@proxmox.com/T/#t

On  2024-11-11  16:50, Aaron Lauterer wrote:
> the first patch adds a new action column to open the task.
> 
> this will hopefully make it clearer for (new) users that a task can be
> opened.
> 
> The second, opional, patch, adapts the code to the more current way of
> opening a new window.
> 
> changes:
> v1: implement code improvement in patch 1/2
> 
> Aaron Lauterer (2):
>    ui: tasks: add actioncolumn to open task
>    ui: tasks: use autoshow instead of win.show
> 
>   www/manager6/dc/Tasks.js | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 



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