* [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment
@ 2023-10-02 15:23 Philipp Hufnagl
2023-10-03 10:34 ` Dominik Csapak
0 siblings, 1 reply; 3+ messages in thread
From: Philipp Hufnagl @ 2023-10-02 15:23 UTC (permalink / raw)
To: pbs-devel
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
---
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;
+}
.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: {
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment
2023-10-02 15:23 [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment Philipp Hufnagl
@ 2023-10-03 10:34 ` Dominik Csapak
2023-10-03 12:37 ` Philipp Hufnagl
0 siblings, 1 reply; 3+ messages in thread
From: Dominik Csapak @ 2023-10-03 10:34 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Philipp Hufnagl
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: {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment
2023-10-03 10:34 ` Dominik Csapak
@ 2023-10-03 12:37 ` Philipp Hufnagl
0 siblings, 0 replies; 3+ messages in thread
From: Philipp Hufnagl @ 2023-10-03 12:37 UTC (permalink / raw)
To: Dominik Csapak, Proxmox Backup Server development discussion
On 10/3/23 12:34, Dominik Csapak wrote:
> 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: {
>
>
I talked with Dominik of list. It seems like I did not reset the
browser cache before. Im really sorry about this
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-03 12:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-02 15:23 [pbs-devel] [PATCH proxmox-backup v4] ui: fix #4260: add dynamic notes in backup group comment Philipp Hufnagl
2023-10-03 10:34 ` Dominik Csapak
2023-10-03 12:37 ` Philipp Hufnagl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox