Re: [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment

[Dominik Csapak <d.csapak@proxmox.com>]

so two comments still (sorry for not catching earlier)
hope these are the last issues i see ;)

On 10/2/23 17:23, Philipp Hufnagl wrote:
> When there is no comment for a backup group, the comment of the last
> snapshot in this group will be shown slightly grayed out as long as
> the group is collapsed.
> 
> Changes since v1:
> * fixed typo at "additonal classes"
> * renamed css class to "faded" so it can be used more generic
> * guarded snapshot comment for not being a string guarded snapshot
> 
> Changes since v2:
> * fixed variable name in violation of coding guidelines
> Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
> 
> Changes since v3:
> * fixed intentation
> * removed span prefix from faded class
> 

please put the changelog below under the '---' else it shows up in the
commit message in git (which we don't need and want)

> ---
>   www/css/ext6-pbs.css     |  3 +++
>   www/datastore/Content.js | 17 ++++++++++++++---
>   2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
> index 5fd65d25..bb239e95 100644
> --- a/www/css/ext6-pbs.css
> +++ b/www/css/ext6-pbs.css
> @@ -226,6 +226,9 @@ span.snapshot-comment-column {
>       display: inline-block;
>       width: calc(100% - 18px);
>   }
> +pmx-faded {
> +    opacity: 0.7;
> +}

two things here: this too could not have worked again, because
you did not define a class here, those are done like this:

---
.pmx-faded { }
---

note the '.' before the classname

i know sometimes testing can be weird, especially frontend stuff,
but this should have been obvious that the text is not a lower
opacity (if the different is not obvious, then maybe the value
is too high or we need a different way to discern these)

it's *really* important that you test your own patches properly,
that way you can catch issues yourself before even sending them to the list,
which decreases the used time from your colleagues that test/review and
increases the chance that the feedback is valuable

also, please leave an empty line between classes in css (so before .pmx-faded)

>   
>   .x-action-col-icon.good:before {
>       color: #21BF4B;
> diff --git a/www/datastore/Content.js b/www/datastore/Content.js
> index 9fc07d49..740d10bf 100644
> --- a/www/datastore/Content.js
> +++ b/www/datastore/Content.js
> @@ -146,6 +146,7 @@ Ext.define('PBS.DataStoreContent', {
>   		    leaf: false,
>   		    iconCls: "fa " + cls,
>   		    expanded: false,
> +		    comment: item.data.comment,
>   		    backup_type: item.data["backup-type"],
>   		    backup_id: item.data["backup-id"],
>   		    children: [],
> @@ -287,6 +288,7 @@ Ext.define('PBS.DataStoreContent', {
>   		    if (item["backup-time"] > last_backup && item.size !== null) {
>   			last_backup = item["backup-time"];
>   			group["backup-time"] = last_backup;
> +			group["last-comment"] = item.comment;
>   			group.files = item.files;
>   			group.size = item.size;
>   			group.owner = item.owner;
> @@ -903,13 +905,22 @@ Ext.define('PBS.DataStoreContent', {
>   		if (!data || data.leaf || data.root) {
>   		    return '';
>   		}
> -		if (v === undefined || v === null) {
> -		    v = '';
> +
> +		let additionalClasses = "";
> +		// when there is no group comment and the section is collapsed,
> +		// display the most recent snapshot comment
> +		if (v === undefined || v === null|| v === '') {
> +		    if (data.expanded === false) {
> +			v = data['last-comment'] ?? '';
> +			additionalClasses = "pmx-faded";
> +		    } else {
> +			v = '';
> +		    }
>   		}
>   		v = Ext.String.htmlEncode(v);
>   		let icon = 'x-action-col-icon fa fa-fw fa-pencil pointer';
>   
> -		return `<span class="snapshot-comment-column">${v}</span>
> +		return `<span class="snapshot-comment-column ${additionalClasses}">${v}</span>
>   		    <i data-qtip="${gettext('Edit')}" style="float: right; margin: 0px;" class="${icon}"></i>`;
>   	    },
>   	    listeners: {