all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #5422: ui: make columns in gcview sortable
Date: Thu, 16 May 2024 11:05:50 +0200	[thread overview]
Message-ID: <20240516090550.m4achavq23mxw7uz@luna.proxmox.com> (raw)
In-Reply-To: <07167532-dee6-490c-b8ac-16bb15a2046f@proxmox.com>

On 15.05.2024 14:34, Thomas Lamprecht wrote:
>Would change subject to:
>
>fix #5422: ui: garbage collection: make columns sortable
>
>Using the class name "gcview" is rather implementation detail and not
>really telling to devs when skimming git log entries or users, if this
>gets copied over into the debian/changelog.

I agree.

>Am 07/05/2024 um 10:13 schrieb Gabriel Goller:
>> These columns were not sortable for some reason. Tested it as well and
>
>The original reason was that PBS did not have a PBS-wide GC/prune view
>until the recently released 3.2, and so each GC view only had a single
>row, as GC jobs are always affecting the whole datastore anyway.
>Thus sorting did not make sense to have for single line entries, showing
>the sort-arrows would only add a tiny bit of confusion for users, as that
>would imply that more entries could be present.

pre-3.2 pbs only used a 1x1 table (so not really a table :) ) for gc jobs, 
so no sorting possible there. I wondered why the patch from Stefan 
Lendl only included the sortable config on some and not all columns :)

>Now for the PBS-wide view sorting makes sense, but for the per-datastore
>GC view it still doesn't.
>
>You could drop the `sortable` config from the columns entries and
>set that dynamically in initComponent, we loop over all columns there
>already and have the information if the view is instantiated for a single
>datastore or for all.
>
>E.g., I'd do something like (untested):
>
>diff --git a/www/config/GCView.js b/www/config/GCView.js
>index 1c9288e4..c3b87fe5 100644
>--- a/www/config/GCView.js
>+++ b/www/config/GCView.js
>@@ -224,12 +224,12 @@ Ext.define('PBS.config.GCJobView', {
>
>     initComponent: function() {
>        let me = this;
>-       let hideLocalDatastore = !!me.datastore;
>+       let isSingleDatastore = !!me.datastore;
>
>        for (let column of me.columns) {
>+           column.sortable = !isSingleDatastore;
>            if (column.dataIndex === 'store') {
>-               column.hidden = hideLocalDatastore;
>-               break;
>+               column.hidden = isSingleDatastore;
>            }
>        }
>
>

Wow, this works like a charm!

>> the sorting seems to work fine.
>>
>
>Could do with a fixes trailer like:
>
>Fixes: db3fd213 ("fix #3217: ui: global prune and gc job view")
>

Will do!

And thanks for the review!


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


      reply	other threads:[~2024-05-16  9:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-07  8:13 Gabriel Goller
2024-05-07 14:11 ` Max Carrara
2024-05-15 12:34 ` Thomas Lamprecht
2024-05-16  9:05   ` Gabriel Goller [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240516090550.m4achavq23mxw7uz@luna.proxmox.com \
    --to=g.goller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal