* [pve-devel] [PATCH common v7 1/2] JSONSchema: refactor tag regex
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-09-20 11:37 ` [pve-devel] applied: " Thomas Lamprecht
2022-06-21 9:19 ` [pve-devel] [PATCH common v7 2/2] JSONSchema: pve-tag: add syntax for 'admin' tags Dominik Csapak
` (24 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
we'll use that elsewhere too
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/JSONSchema.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index ab718f3..54c149d 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -696,12 +696,14 @@ register_standard_option('proxmox-remote', {
type => 'string', format => 'proxmox-remote',
});
+our $PVE_TAG_RE = qr/[a-z0-9_][a-z0-9_\-\+\.]*/i;
+
# used for pve-tag-list in e.g., guest configs
register_format('pve-tag', \&pve_verify_tag);
sub pve_verify_tag {
my ($value, $noerr) = @_;
- return $value if $value =~ m/^[a-z0-9_][a-z0-9_\-\+\.]*$/i;
+ return $value if $value =~ m/^${PVE_TAG_RE}$/i;
return undef if $noerr;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH common v7 2/2] JSONSchema: pve-tag: add syntax for 'admin' tags
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH common v7 1/2] JSONSchema: refactor tag regex Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method Dominik Csapak
` (23 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
by prefixing the tag with '+'
these can be useful to distinguish tags set by a 'normal' user, and an
admin. This patch is only useful with additional patches that
check those permissions while setting the tags though.
i chose a syntax that was invalid before, but does not add
unnecessary overhead. Additionally, the character was allowed (but not
as first), so there should problem arise from that.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/JSONSchema.pm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 54c149d..bf2c9ca 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -696,7 +696,7 @@ register_standard_option('proxmox-remote', {
type => 'string', format => 'proxmox-remote',
});
-our $PVE_TAG_RE = qr/[a-z0-9_][a-z0-9_\-\+\.]*/i;
+our $PVE_TAG_RE = qr/\+?[a-z0-9_][a-z0-9_\-\+\.]*/i;
# used for pve-tag-list in e.g., guest configs
register_format('pve-tag', \&pve_verify_tag);
@@ -710,6 +710,13 @@ sub pve_verify_tag {
die "invalid characters in tag\n";
}
+sub is_admin_pve_tag {
+ my ($tag) = @_;
+ return undef if !defined($tag);
+ return 1 if $tag =~ m/^\+/;
+ return 0;
+}
+
sub pve_parse_startup_order {
my ($value) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH cluster v7 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH common v7 1/2] JSONSchema: refactor tag regex Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH common v7 2/2] JSONSchema: pve-tag: add syntax for 'admin' tags Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 2/3] Cluster: add get_guest_config_properties Dominik Csapak
` (22 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
for getting multiple properties from the in memory config of the
guests in one go. Keep the existing IPC call as is for backward
compatibility and add this as separate, new one.
It basically behaves the same as
CFS_IPC_GET_GUEST_CONFIG_PROPERTY, but takes a list of properties
instead and returns multiple properties per guest.
The old way of getting a single property is now also done by
the new function, since it basically performs identically.
Here a short benchmark:
Setup:
PVE in a VM with cpu type host (12700k) and 4 cores
10000 typical configs with both 'lock' and 'tags' set at the end
and fairly long tags ('test-tag1,test-tag2,test-tag3')
(normal vm with a snapshot, ~1KiB)
i let it run 100 times each, times in ms
old code:
total time time per iteration
1054.2 10.2
new code:
num props total time time per iter function
2 1332.2 13.2 get_properties
1 1051.2 10.2 get_properties
2 2082.2 20.2 get_property (2 calls to get_property)
1 1034.2 10.2 get_property
so a call with the new code for one property is the same as with the
old code, and adding a second property only adds a bit of additional
time (in this case ~30%)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
data/src/cfs-ipc-ops.h | 2 +
data/src/server.c | 64 +++++++++++++++
data/src/status.c | 177 ++++++++++++++++++++++++++++-------------
data/src/status.h | 3 +
4 files changed, 190 insertions(+), 56 deletions(-)
diff --git a/data/src/cfs-ipc-ops.h b/data/src/cfs-ipc-ops.h
index 003e233..249308d 100644
--- a/data/src/cfs-ipc-ops.h
+++ b/data/src/cfs-ipc-ops.h
@@ -43,4 +43,6 @@
#define CFS_IPC_VERIFY_TOKEN 12
+#define CFS_IPC_GET_GUEST_CONFIG_PROPERTIES 13
+
#endif
diff --git a/data/src/server.c b/data/src/server.c
index 549788a..ced9cfc 100644
--- a/data/src/server.c
+++ b/data/src/server.c
@@ -89,6 +89,13 @@ typedef struct {
char property[];
} cfs_guest_config_propery_get_request_header_t;
+typedef struct {
+ struct qb_ipc_request_header req_header;
+ uint32_t vmid;
+ uint8_t num_props;
+ char props[]; /* list of \0 terminated properties */
+} cfs_guest_config_properties_get_request_header_t;
+
typedef struct {
struct qb_ipc_request_header req_header;
char token[];
@@ -348,6 +355,63 @@ static int32_t s1_msg_process_fn(
result = cfs_create_guest_conf_property_msg(outbuf, memdb, rh->property, rh->vmid);
}
+ } else if (request_id == CFS_IPC_GET_GUEST_CONFIG_PROPERTIES) {
+
+ cfs_guest_config_properties_get_request_header_t *rh =
+ (cfs_guest_config_properties_get_request_header_t *) data;
+
+ size_t remaining = request_size - G_STRUCT_OFFSET(cfs_guest_config_properties_get_request_header_t, props);
+
+ result = 0;
+ if (rh->vmid < 100 && rh->vmid != 0) {
+ cfs_debug("vmid out of range %u", rh->vmid);
+ result = -EINVAL;
+ } else if (rh->vmid >= 100 && !vmlist_vm_exists(rh->vmid)) {
+ result = -ENOENT;
+ } else if (rh->num_props == 0) {
+ cfs_debug("num_props == 0");
+ result = -EINVAL;
+ } else if (remaining <= 1) {
+ cfs_debug("property length <= 1, %ld", remaining);
+ result = -EINVAL;
+ } else {
+ const char **properties = malloc(sizeof(char*) * rh->num_props);
+ char *current = (rh->props);
+ for (uint8_t i = 0; i < rh->num_props; i++) {
+ size_t proplen = strnlen(current, remaining);
+ if (proplen == 0) {
+ cfs_debug("property length 0");
+ result = -EINVAL;
+ break;
+ }
+ if (proplen == remaining) {
+ cfs_debug("property not \\0 terminated");
+ result = -EINVAL;
+ break;
+ }
+ if (current[0] < 'a' || current[0] > 'z') {
+ cfs_debug("property does not start with [a-z]");
+ result = -EINVAL;
+ break;
+ }
+ properties[i] = current;
+ current[proplen] = '\0'; // ensure property is 0 terminated
+ remaining -= (proplen + 1);
+ current += proplen + 1;
+ }
+
+ if (remaining != 0) {
+ cfs_debug("leftover data after parsing %u properties", rh->num_props);
+ result = -EINVAL;
+ }
+
+ if (result == 0) {
+ cfs_debug("cfs_get_guest_config_properties: basic valid checked, do request");
+ result = cfs_create_guest_conf_properties_msg(outbuf, memdb, properties, rh->num_props, rh->vmid);
+ }
+
+ free(properties);
+ }
} else if (request_id == CFS_IPC_VERIFY_TOKEN) {
cfs_verify_token_request_header_t *rh = (cfs_verify_token_request_header_t *) data;
diff --git a/data/src/status.c b/data/src/status.c
index 9bceaeb..e2bedaa 100644
--- a/data/src/status.c
+++ b/data/src/status.c
@@ -804,25 +804,52 @@ cfs_create_vmlist_msg(GString *str)
return 0;
}
-// checks the conf for a line starting with '$prop:' and returns the value
-// afterwards, whitout initial whitespace(s), we only deal with the format
-// restricion imposed by our perl VM config parser, main reference is
+// checks if a config line starts with the given prop. if yes, writes a '\0'
+// at the end of the value, and returns the pointer where the value starts
+char *
+_get_property_value_from_line(char *line, int line_len, const char *prop, int prop_len)
+{
+ if (line_len <= prop_len + 1) return NULL;
+
+ if (line[prop_len] == ':' && memcmp(line, prop, prop_len) == 0) { // found
+ char *v_start = &line[prop_len + 1];
+
+ // drop initial value whitespaces here already
+ while (*v_start && isspace(*v_start)) v_start++;
+
+ if (!*v_start) return NULL;
+
+ char *v_end = &line[line_len - 1];
+ while (v_end > v_start && isspace(*v_end)) v_end--;
+ v_end[1] = '\0';
+
+ return v_start;
+ }
+
+ return NULL;
+}
+
+// checks the conf for lines starting with the given props and
+// writes the pointers into the correct positions into the 'found' array
+// afterwards, without initial whitespace(s), we only deal with the format
+// restriction imposed by our perl VM config parser, main reference is
// PVE::QemuServer::parse_vm_config this allows to be very fast and still
// relatively simple
-// main restrictions used for our advantage is the properties match reges:
+// main restrictions used for our advantage is the properties match regex:
// ($line =~ m/^([a-z][a-z_]*\d*):\s*(.+?)\s*$/) from parse_vm_config
// currently we only look at the current configuration in place, i.e., *no*
-// snapshort and *no* pending changes
-static char *
-_get_property_value(char *conf, int conf_size, const char *prop, int prop_len)
+// snapshot and *no* pending changes
+void
+_get_property_values(char **found, char *conf, int conf_size, const char **props, uint8_t num_props, char min, char max)
{
const char *const conf_end = conf + conf_size;
char *line = conf;
- size_t remaining_size;
+ size_t remaining_size = conf_size;
+ int count = 0;
char *next_newline = memchr(conf, '\n', conf_size);
if (next_newline == NULL) {
- return NULL; // valid property lines end with \n, but none in the config
+ return; // valid property lines end with \n, but none in the config
}
*next_newline = '\0';
@@ -830,41 +857,32 @@ _get_property_value(char *conf, int conf_size, const char *prop, int prop_len)
if (!line[0]) goto next;
// snapshot or pending section start, but nothing found yet -> not found
- if (line[0] == '[') return NULL;
- // properties start with /^[a-z]/, so continue early if not
- if (line[0] < 'a' || line[0] > 'z') goto next;
+ if (line[0] == '[') return;
+ // continue early if line does not begin with the min/max char of the properties
+ if (line[0] < min || line[0] > max) goto next;
int line_len = strlen(line);
- if (line_len <= prop_len + 1) goto next;
-
- if (line[prop_len] == ':' && memcmp(line, prop, prop_len) == 0) { // found
- char *v_start = &line[prop_len + 1];
-
- // drop initial value whitespaces here already
- while (*v_start && isspace(*v_start)) v_start++;
-
- if (!*v_start) return NULL;
-
- char *v_end = &line[line_len - 1];
- while (v_end > v_start && isspace(*v_end)) v_end--;
- v_end[1] = '\0';
-
- return v_start;
+ for (uint8_t i = 0; i < num_props; i++) {
+ char * value = _get_property_value_from_line(line, line_len, props[i], strlen(props[i]));
+ if (value != NULL) {
+ count += (found[i] != NULL) & 0x1; // count newly found lines
+ found[i] = value;
+ }
+ }
+ if (count == num_props) {
+ return; // found all
}
next:
line = next_newline + 1;
remaining_size = conf_end - line;
- if (remaining_size <= prop_len) {
- return NULL;
- }
next_newline = memchr(line, '\n', remaining_size);
if (next_newline == NULL) {
- return NULL; // valid property lines end with \n, but none in the config
+ return; // valid property lines end with \n, but none in the config
}
*next_newline = '\0';
}
- return NULL; // not found
+ return;
}
static void
@@ -883,24 +901,77 @@ _g_str_append_kv_jsonescaped(GString *str, const char *k, const char *v)
}
int
-cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid)
+_print_found_properties(
+ GString *str,
+ gpointer conf,
+ int size,
+ const char **props,
+ uint8_t num_props,
+ uint32_t vmid,
+ char **values,
+ char min,
+ char max,
+ int first)
+{
+ _get_property_values(values, conf, size, props, num_props, min, max);
+
+ uint8_t found = 0;
+ for (uint8_t i = 0; i < num_props; i++) {
+ if (values[i] == NULL) {
+ continue;
+ }
+ if (found) {
+ g_string_append_c(str, ',');
+ } else {
+ if (!first) {
+ g_string_append_printf(str, ",\n");
+ } else {
+ first = 0;
+ }
+ g_string_append_printf(str, "\"%u\":{", vmid);
+ found = 1;
+ }
+ _g_str_append_kv_jsonescaped(str, props[i], values[i]);
+ }
+
+ if (found) {
+ g_string_append_c(str, '}');
+ }
+
+ return first;
+}
+
+int
+cfs_create_guest_conf_properties_msg(GString *str, memdb_t *memdb, const char **props, uint8_t num_props, uint32_t vmid)
{
g_return_val_if_fail(cfs_status.vmlist != NULL, -EINVAL);
g_return_val_if_fail(str != NULL, -EINVAL);
- int prop_len = strlen(prop);
- int res = 0;
- GString *path = NULL;
-
// Prelock &memdb->mutex in order to enable the usage of memdb_read_nolock
// to prevent Deadlocks as in #2553
g_mutex_lock (&memdb->mutex);
g_mutex_lock (&mutex);
- g_string_printf(str,"{\n");
+ g_string_printf(str, "{\n");
GHashTable *ht = cfs_status.vmlist;
+
+ int res = 0;
+ GString *path = NULL;
gpointer tmp = NULL;
+ char **values = calloc(num_props, sizeof(char*));
+ char min = 'z', max = 'a';
+
+ for (uint8_t i = 0; i < num_props; i++) {
+ if (props[i][0] > max) {
+ max = props[i][0];
+ }
+
+ if (props[i][0] < min) {
+ min = props[i][0];
+ }
+ }
+
if (!g_hash_table_size(ht)) {
goto ret;
}
@@ -919,15 +990,8 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *pro
// use memdb_read_nolock because lock is handled here
int size = memdb_read_nolock(memdb, path->str, &tmp);
if (tmp == NULL) goto err;
- if (size <= prop_len) goto ret;
-
- char *val = _get_property_value(tmp, size, prop, prop_len);
- if (val == NULL) goto ret;
-
- g_string_append_printf(str, "\"%u\":{", vmid);
- _g_str_append_kv_jsonescaped(str, prop, val);
- g_string_append_c(str, '}');
+ _print_found_properties(str, tmp, size, props, num_props, vmid, values, min, max, 1);
} else {
GHashTableIter iter;
g_hash_table_iter_init (&iter, ht);
@@ -943,21 +1007,16 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *pro
tmp = NULL;
// use memdb_read_nolock because lock is handled here
int size = memdb_read_nolock(memdb, path->str, &tmp);
- if (tmp == NULL || size <= prop_len) continue;
+ if (tmp == NULL) continue;
- char *val = _get_property_value(tmp, size, prop, prop_len);
- if (val == NULL) continue;
-
- if (!first) g_string_append_printf(str, ",\n");
- else first = 0;
-
- g_string_append_printf(str, "\"%u\":{", vminfo->vmid);
- _g_str_append_kv_jsonescaped(str, prop, val);
- g_string_append_c(str, '}');
+ memset(values, 0, sizeof(char*)*num_props); // reset array
+ first = _print_found_properties(str, tmp, size, props, num_props,
+ vminfo->vmid, values, min, max, first);
}
}
ret:
g_free(tmp);
+ free(values);
if (path != NULL) {
g_string_free(path, TRUE);
}
@@ -973,6 +1032,12 @@ enoent:
goto ret;
}
+int
+cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid)
+{
+ return cfs_create_guest_conf_properties_msg(str, memdb, &prop, 1, vmid);
+}
+
void
record_memdb_change(const char *path)
{
diff --git a/data/src/status.h b/data/src/status.h
index bbf0948..041cb34 100644
--- a/data/src/status.h
+++ b/data/src/status.h
@@ -163,4 +163,7 @@ cfs_create_memberlist_msg(
int
cfs_create_guest_conf_property_msg(GString *str, memdb_t *memdb, const char *prop, uint32_t vmid);
+int
+cfs_create_guest_conf_properties_msg(GString *str, memdb_t *memdb, const char **props, uint8_t num_props, uint32_t vmid);
+
#endif /* _PVE_STATUS_H_ */
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH cluster v7 2/3] Cluster: add get_guest_config_properties
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (2 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 1/3] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 3/3] datacenter.cfg: add option for tag-style Dominik Csapak
` (21 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
akin to get_guest_config_property, but with a list of properties.
uses the new CFS_IPC_GET_GUEST_CONFIG_PROPERTIES
also adds the same NOTEs regarding parsing/permissions to the comment
of get_guest_config_property
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
data/PVE/Cluster.pm | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
index 05451fd..d0148fc 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -340,10 +340,37 @@ sub get_node_kv {
return $res;
}
+# properties: an array-ref of config properties you want to get, e.g., this
+# is perfect to get multiple properties of a guest _fast_
+# (>100 faster than manual parsing here)
+# vmid: optional, if a valid is passed we only check that one, else return all
+# NOTE: does *not* searches snapshot and PENDING entries sections!
+# NOTE: returns the guest config lines (excluding trailing whitespace) as is,
+# so for non-trivial properties, checking the validity must be done
+# NOTE: no permission check is done, that is the responsibilty of the caller
+sub get_guest_config_properties {
+ my ($properties, $vmid) = @_;
+
+ die "properties required" if !defined($properties);
+
+ my $num_props = scalar(@$properties);
+ die "only up to 255 properties supported" if $num_props > 255;
+ my $bindata = pack "VC", $vmid // 0, $num_props;
+ for my $property (@$properties) {
+ $bindata .= pack "Z*", $property;
+ }
+ my $res = $ipcc_send_rec_json->(CFS_IPC_GET_GUEST_CONFIG_PROPERTIES, $bindata);
+
+ return $res;
+}
+
# property: a config property you want to get, e.g., this is perfect to get
# the 'lock' entry of a guest _fast_ (>100 faster than manual parsing here)
# vmid: optional, if a valid is passed we only check that one, else return all
# NOTE: does *not* searches snapshot and PENDING entries sections!
+# NOTE: returns the guest config lines (excluding trailing whitespace) as is,
+# so for non-trivial properties, checking the validity must be done
+# NOTE: no permission check is done, that is the responsibilty of the caller
sub get_guest_config_property {
my ($property, $vmid) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH cluster v7 3/3] datacenter.cfg: add option for tag-style
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (3 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 2/3] Cluster: add get_guest_config_properties Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 1/3] add tag related helpers Dominik Csapak
` (20 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
its a property string containing 'tree-shape' and 'colors'
the colors are formatted like this:
<tag>:<background-color>[:<text-color>]
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
data/PVE/DataCenterConfig.pm | 37 ++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/data/PVE/DataCenterConfig.pm b/data/PVE/DataCenterConfig.pm
index bcd77f8..30db37c 100644
--- a/data/PVE/DataCenterConfig.pm
+++ b/data/PVE/DataCenterConfig.pm
@@ -125,6 +125,29 @@ sub pve_verify_mac_prefix {
return $mac_prefix;
}
+my $COLOR_RE = '[0-9a-fA-F]{6}';
+my $TAG_COLOR_OVERRIDE_RE = "(?:${PVE::JSONSchema::PVE_TAG_RE}:${COLOR_RE}(?:\:${COLOR_RE})?)";
+
+my $tag_style_format = {
+ 'tree-shape' => {
+ optional => 1,
+ type => 'string',
+ enum => ['full', 'circle', 'dense', 'none'],
+ default => 'circle',
+ description => "Tag style in tree. 'full' draws the full tag. 'circle' ".
+ "draws only a circle with the background color. 'dense' only draws ".
+ "a small rectancle (useful when many tags are assigned to each guest).".
+ "'none' disables showing the tags.",
+ },
+ 'colors' => {
+ optional => 1,
+ type => 'string',
+ pattern => "${TAG_COLOR_OVERRIDE_RE}(?:\;$TAG_COLOR_OVERRIDE_RE)*",
+ typetext => '<tag>:<hex-color>[:<hex-color-for-text>][;<tag>=...]',
+ description => "Manual color mapping for tags (semicolon separated).",
+ },
+};
+
my $datacenter_schema = {
type => "object",
additionalProperties => 0,
@@ -250,6 +273,12 @@ my $datacenter_schema = {
maxLength => 64 * 1024,
optional => 1,
},
+ 'tag-style' => {
+ optional => 1,
+ type => 'string',
+ description => "Tag style options.",
+ format => $tag_style_format,
+ },
},
};
@@ -294,6 +323,10 @@ sub parse_datacenter_config {
$res->{webauthn} = parse_property_string($webauthn_format, $webauthn);
}
+ if (my $tag_style = $res->{'tag-style'}) {
+ $res->{'tag-style'} = parse_property_string($tag_style_format, $tag_style);
+ }
+
# for backwards compatibility only, new migration property has precedence
if (defined($res->{migration_unsecure})) {
if (defined($res->{migration}->{type})) {
@@ -353,6 +386,10 @@ sub write_datacenter_config {
$cfg->{webauthn} = PVE::JSONSchema::print_property_string($webauthn, $webauthn_format);
}
+ if (ref(my $tag_style = $cfg->{'tag-style'})) {
+ $cfg->{'tag-style'} = PVE::JSONSchema::print_property_string($tag_style, $tag_style_format);
+ }
+
my $comment = '';
# add description as comment to top of file
my $description = $cfg->{description} || '';
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH widget-toolkit v7 1/3] add tag related helpers
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (4 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH cluster v7 3/3] datacenter.cfg: add option for tag-style Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 2/3] add class for 'admin' tags Dominik Csapak
` (19 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
helpers to
* generate a color from a string consistently
* generate a html tag for a tag
* related css classes
contrast is calculated according to SAPC draft:
https://github.com/Myndex/SAPC-APCA
which is likely to become a w3c guideline in the future and seems
to be a better algorithm for this
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/Utils.js | 90 ++++++++++++++++++++++++++++++++++++++++++++
src/css/ext6-pmx.css | 45 ++++++++++++++++++++++
2 files changed, 135 insertions(+)
diff --git a/src/Utils.js b/src/Utils.js
index 6a03057..eb13838 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -1272,6 +1272,96 @@ utilities: {
.map(val => val.charCodeAt(0)),
);
},
+
+ stringToRGB: function(string) {
+ let hash = 0;
+ if (!string) {
+ return hash;
+ }
+ string += 'prox'; // give short strings more variance
+ for (let i = 0; i < string.length; i++) {
+ hash = string.charCodeAt(i) + ((hash << 5) - hash);
+ hash = hash & hash; // to int
+ }
+
+ let alpha = 0.7; // make the color a bit brighter
+ let bg = 255; // assume white background
+
+ return [
+ (hash & 255)*alpha + bg*(1-alpha),
+ ((hash >> 8) & 255)*alpha + bg*(1-alpha),
+ ((hash >> 16) & 255)*alpha + bg*(1-alpha),
+ ];
+ },
+
+ rgbToCss: function(rgb) {
+ return `rgb(${rgb[0]}, ${rgb[1]}, ${rgb[2]})`;
+ },
+
+ rgbToHex: function(rgb) {
+ let r = Math.round(rgb[0]).toString(16);
+ let g = Math.round(rgb[1]).toString(16);
+ let b = Math.round(rgb[2]).toString(16);
+ return `${r}${g}${b}`;
+ },
+
+ hexToRGB: function(hex) {
+ if (!hex) {
+ return undefined;
+ }
+ if (hex.length === 7) {
+ hex = hex.slice(1);
+ }
+ let r = parseInt(hex.slice(0, 2), 16);
+ let g = parseInt(hex.slice(2, 4), 16);
+ let b = parseInt(hex.slice(4, 6), 16);
+ return [r, g, b];
+ },
+
+ // optimized & simplified SAPC function
+ // https://github.com/Myndex/SAPC-APCA
+ getTextContrastClass: function(rgb) {
+ const blkThrs = 0.022;
+ const blkClmp = 1.414;
+
+ // linearize & gamma correction
+ let r = (rgb[0]/255)**2.4;
+ let g = (rgb[1]/255)**2.4;
+ let b = (rgb[2]/255)**2.4;
+
+ // relative luminance sRGB
+ let bg = r*0.2126729 + g*0.7151522 + b*0.0721750;
+
+ // black clamp
+ bg = bg > blkThrs ? bg : bg + (blkThrs - bg) ** blkClmp;
+
+ // SAPC with white text
+ let contrastLight = bg ** 0.65 - 1;
+ // SAPC with black text
+ let contrastDark = bg ** 0.56 - 0.046134502;
+
+ if (Math.abs(contrastLight) >= Math.abs(contrastDark)) {
+ return 'light';
+ } else {
+ return 'dark';
+ }
+ },
+
+ getTagElement: function(string, color_overrides) {
+ let rgb = color_overrides?.[string] || Proxmox.Utils.stringToRGB(string);
+ let bgcolor = Proxmox.Utils.rgbToCss(rgb);
+ let style = `background-color: ${bgcolor};`;
+ let cls;
+ if (rgb.length > 3) {
+ let fgcolor = Proxmox.Utils.rgbToCss([rgb[3], rgb[4], rgb[5]]);
+ style += `color: ${fgcolor}`;
+ cls = "proxmox-tag-dark";
+ } else {
+ let txtCls = Proxmox.Utils.getTextContrastClass(rgb);
+ cls = `proxmox-tag-${txtCls}`;
+ }
+ return `<span class="${cls}" style="${style}">${string}</span>`;
+ },
},
singleton: true,
diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css
index 886e24a..3e0f04f 100644
--- a/src/css/ext6-pmx.css
+++ b/src/css/ext6-pmx.css
@@ -6,6 +6,51 @@
background-color: LightYellow;
}
+.proxmox-tags-full .proxmox-tag-light,
+.proxmox-tags-full .proxmox-tag-dark {
+ border-radius: 3px;
+ padding: 1px 6px;
+ margin: 0px 1px;
+}
+
+.proxmox-tags-circle .proxmox-tag-light,
+.proxmox-tags-circle .proxmox-tag-dark {
+ margin: 0px 1px;
+ position: relative;
+ top: 2px;
+ border-radius: 6px;
+ height: 12px;
+ width: 12px;
+ display: inline-block;
+ color: transparent !important;
+ overflow: hidden;
+}
+
+.proxmox-tags-none .proxmox-tag-light,
+.proxmox-tags-none .proxmox-tag-dark {
+ display: none;
+}
+
+.proxmox-tags-dense .proxmox-tag-light,
+.proxmox-tags-dense .proxmox-tag-dark {
+ width: 6px;
+ margin-right: 1px;
+ display: inline-block;
+ color: transparent !important;
+ overflow: hidden;
+ vertical-align: bottom;
+}
+
+.proxmox-tags-full .proxmox-tag-light {
+ color: #fff;
+ background-color: #383838;
+}
+
+.proxmox-tags-full .proxmox-tag-dark {
+ color: #000;
+ background-color: #f0f0f0;
+}
+
.x-mask-msg-text {
text-align: center;
}
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH widget-toolkit v7 1/3] add tag related helpers
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 1/3] add tag related helpers Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
0 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Some small nits inline
On 6/21/22 11:19, Dominik Csapak wrote:
> helpers to
> * generate a color from a string consistently
> * generate a html tag for a tag
> * related css classes
>
> contrast is calculated according to SAPC draft:
> https://github.com/Myndex/SAPC-APCA
>
> which is likely to become a w3c guideline in the future and seems
> to be a better algorithm for this
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/Utils.js | 90 ++++++++++++++++++++++++++++++++++++++++++++
> src/css/ext6-pmx.css | 45 ++++++++++++++++++++++
> 2 files changed, 135 insertions(+)
>
> diff --git a/src/Utils.js b/src/Utils.js
> index 6a03057..eb13838 100644
> --- a/src/Utils.js
> +++ b/src/Utils.js
> @@ -1272,6 +1272,96 @@ utilities: {
> .map(val => val.charCodeAt(0)),
> );
> },
> +
> + stringToRGB: function(string) {
> + let hash = 0;
> + if (!string) {
> + return hash;
> + }
> + string += 'prox'; // give short strings more variance
> + for (let i = 0; i < string.length; i++) {
> + hash = string.charCodeAt(i) + ((hash << 5) - hash);
> + hash = hash & hash; // to int
> + }
> +
> + let alpha = 0.7; // make the color a bit brighter
> + let bg = 255; // assume white background
> +
> + return [
> + (hash & 255)*alpha + bg*(1-alpha),
> + ((hash >> 8) & 255)*alpha + bg*(1-alpha),
> + ((hash >> 16) & 255)*alpha + bg*(1-alpha),
I don't think our style guides specify this clearly, but I find the mix of
spaces and no spaces around the operators inconsistent. There are a few more
places in this patch where we do have that kind of inconsistency.
> + ];
> + },
> +
> + rgbToCss: function(rgb) {
> + return `rgb(${rgb[0]}, ${rgb[1]}, ${rgb[2]})`;
> + },
> +
> + rgbToHex: function(rgb) {
> + let r = Math.round(rgb[0]).toString(16);
> + let g = Math.round(rgb[1]).toString(16);
> + let b = Math.round(rgb[2]).toString(16);
> + return `${r}${g}${b}`;
> + },
[...]
> +
> + getTagElement: function(string, color_overrides) {
> + let rgb = color_overrides?.[string] || Proxmox.Utils.stringToRGB(string);
> + let bgcolor = Proxmox.Utils.rgbToCss(rgb);
> + let style = `background-color: ${bgcolor};`;
Couldn't we save a line by calling Proxmox.Utils.rgbToCss directly in the
string? E.g.
let style = `background-color: ${Proxmox.Utils.rgbToCss(rgb)};`;
> + let cls;
> + if (rgb.length > 3) {
> + let fgcolor = Proxmox.Utils.rgbToCss([rgb[3], rgb[4], rgb[5]]);
> + style += `color: ${fgcolor}`;
Same as above here.
> + cls = "proxmox-tag-dark";
> + } else {
> + let txtCls = Proxmox.Utils.getTextContrastClass(rgb);
> + cls = `proxmox-tag-${txtCls}`;
> + }
> + return `<span class="${cls}" style="${style}">${string}</span>`;
> + },
> },
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH widget-toolkit v7 2/3] add class for 'admin' tags
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (5 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 1/3] add tag related helpers Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 3/3] Toolkit: add override for Ext.dd.DragDropManager Dominik Csapak
` (18 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
when a tag starts with '+', we want to emphasize it, so it's clear that
it's an 'admin' tag, do this by making the font bold
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/Utils.js | 4 ++++
src/css/ext6-pmx.css | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/src/Utils.js b/src/Utils.js
index eb13838..7d490c0 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -1360,6 +1360,10 @@ utilities: {
let txtCls = Proxmox.Utils.getTextContrastClass(rgb);
cls = `proxmox-tag-${txtCls}`;
}
+
+ if (string[0] === '+') {
+ cls += ' admin-tag';
+ }
return `<span class="${cls}" style="${style}">${string}</span>`;
},
},
diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css
index 3e0f04f..486de18 100644
--- a/src/css/ext6-pmx.css
+++ b/src/css/ext6-pmx.css
@@ -6,6 +6,10 @@
background-color: LightYellow;
}
+.admin-tag {
+ font-weight: bold;
+}
+
.proxmox-tags-full .proxmox-tag-light,
.proxmox-tags-full .proxmox-tag-dark {
border-radius: 3px;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH widget-toolkit v7 3/3] Toolkit: add override for Ext.dd.DragDropManager
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (6 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 2/3] add class for 'admin' tags Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges Dominik Csapak
` (17 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
to fix selection behavior for Ext.dd.DragZone.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/Toolkit.js | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/Toolkit.js b/src/Toolkit.js
index a1d291e..b6eacfb 100644
--- a/src/Toolkit.js
+++ b/src/Toolkit.js
@@ -699,6 +699,22 @@ Ext.define('Proxmox.view.DragZone', {
},
});
+// Fix text selection on drag when using DragZone,
+// see https://forum.sencha.com/forum/showthread.php?335100
+Ext.define('Proxmox.dd.DragDropManager', {
+ override: 'Ext.dd.DragDropManager',
+
+ stopEvent: function(e) {
+ if (this.stopPropagation) {
+ e.stopPropagation();
+ }
+
+ if (this.preventDefault) {
+ e.preventDefault();
+ }
+ },
+});
+
// force alert boxes to be rendered with an Error Icon
// since Ext.Msg is an object and not a prototype, we need to override it
// after the framework has been initiated
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (7 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH widget-toolkit v7 3/3] Toolkit: add override for Ext.dd.DragDropManager Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:19 ` [pve-devel] [PATCH container v7 1/1] check_ct_modify_config_perm: " Dominik Csapak
` (16 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
normal tags require 'VM.Config.Options' on the VM, admin tags require
'Sys.Modify' on '/'
a user can set/delete/reorder tags, as long as no admin tags get
added/removed
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/API2/Qemu.pm | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
PVE/QemuServer.pm | 3 ++-
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 99b426e..37b56c8 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -519,7 +519,6 @@ my $generaloptions = {
'startup' => 1,
'tdf' => 1,
'template' => 1,
- 'tags' => 1,
};
my $vmpoweroptions = {
@@ -589,6 +588,7 @@ my $check_vm_modify_config_perm = sub {
next if PVE::QemuServer::is_valid_drivename($opt);
next if $opt eq 'cdrom';
next if $opt =~ m/^(?:unused|serial|usb)\d+$/;
+ next if $opt eq 'tags';
if ($cpuoptions->{$opt} || $opt =~ m/^numa\d+$/) {
@@ -1559,6 +1559,27 @@ my $update_vm_api = sub {
}
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
+ } elsif ($opt eq 'tags') {
+ my $check_admin = 0;
+ my $check_options = 0;
+ foreach my $tag (PVE::Tools::split_list($val)) {
+ if (PVE::JSONSchema::is_admin_pve_tag($tag)) {
+ $check_admin = 1;
+ } else {
+ $check_options = 1;
+ }
+ }
+
+ if ($check_admin) {
+ $rpcenv->check($authuser, "/", ['Sys.Modify']);
+ }
+
+ if ($check_options) {
+ $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Options']);
+ }
+
+ delete $conf->{$opt};
+ PVE::QemuConfig->write_config($vmid, $conf);
} else {
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
@@ -1619,6 +1640,39 @@ my $update_vm_api = sub {
die "only root can modify '$opt' config for real devices\n";
}
$conf->{pending}->{$opt} = $param->{$opt};
+ } elsif ($opt eq 'tags') {
+ my $check_admin = 0;
+ my $check_user = 0;
+
+ my $old_tags = {};
+ my $new_tags = {};
+
+ map { $old_tags->{$_} = 1 } PVE::Tools::split_list($conf->{$opt});
+ map { $new_tags->{$_} = 1 } PVE::Tools::split_list($param->{$opt});
+
+ my $check_tags = sub {
+ my ($a, $b) = @_;
+ foreach my $tag (keys %$a) {
+ next if $b->{$tag};
+ if (PVE::JSONSchema::is_admin_pve_tag($tag)) {
+ $check_admin = 1;
+ } else {
+ $check_user = 1;
+ }
+ }
+ };
+
+ $check_tags->($old_tags, $new_tags);
+ $check_tags->($new_tags, $old_tags);
+
+ if ($check_admin) {
+ $rpcenv->check($authuser, "/", ['VM.Config.Options']);
+ }
+
+ if ($check_user) {
+ $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Options']);
+ }
+ $conf->{pending}->{$opt} = $param->{$opt};
} else {
$conf->{pending}->{$opt} = $param->{$opt};
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b3964bc..48247cc 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -698,7 +698,8 @@ EODESCR
},
tags => {
type => 'string', format => 'pve-tag-list',
- description => 'Tags of the VM. This is only meta information.',
+ description => 'Tags of the VM. This is only meta information. Prefixing a tag with'.
+ "'+' marks it as an admin tag and can only be set with 'Sys.Modify' on '/'.",
optional => 1,
},
rng0 => {
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges
2022-06-21 9:19 ` [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
2022-09-15 11:46 ` Dominik Csapak
0 siblings, 1 reply; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Something that crossed my mind:
Have you thought about not allowing tags if they match an admin tag, except for
the '+'?
Depending on what they will be used for in the future, there could be some
potential to trick an admin by creating a similar regular tag. Any code relying
on admin tags should not have an issue with that, but even though the color in
the GUI should be different, one could try to trick an admin to do something
they should not, depending on the tags.
Visual spoofing with similar looking UTF8 characters should not be much of an
issue, due to the regex used.
On 6/21/22 11:19, Dominik Csapak wrote:
> normal tags require 'VM.Config.Options' on the VM, admin tags require
> 'Sys.Modify' on '/'
>
> a user can set/delete/reorder tags, as long as no admin tags get
> added/removed
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges
2022-09-14 14:15 ` Aaron Lauterer
@ 2022-09-15 11:46 ` Dominik Csapak
0 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-09-15 11:46 UTC (permalink / raw)
To: Aaron Lauterer, Proxmox VE development discussion
On 9/14/22 16:15, Aaron Lauterer wrote:
> Something that crossed my mind:
>
> Have you thought about not allowing tags if they match an admin tag, except for the '+'?
> Depending on what they will be used for in the future, there could be some potential to trick an
> admin by creating a similar regular tag. Any code relying on admin tags should not have an issue
> with that, but even though the color in the GUI should be different, one could try to trick an admin
> to do something they should not, depending on the tags.
> Visual spoofing with similar looking UTF8 characters should not be much of an issue, due to the
> regex used.
>
>
i get what you mean, but it's difficult to implement. in the current version,
we only ever have the tags currently defined, not the global defined ones.
alternatively we could let an admin define a set of admin tags in the cluster,
which could then be off-limits for setting/removing for non-admins
that would potentially also solve the problem of having a seperate regex
for them in the first place
as for confusion: admin tags always are prefixed with a '+' symbol currently
so, imho '+backup' and 'backup' are different enough?
> On 6/21/22 11:19, Dominik Csapak wrote:
>> normal tags require 'VM.Config.Options' on the VM, admin tags require
>> 'Sys.Modify' on '/'
>>
>> a user can set/delete/reorder tags, as long as no admin tags get
>> added/removed
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>
>
> [...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH container v7 1/1] check_ct_modify_config_perm: check 'admin' tags privileges
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (8 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-06-21 9:19 ` [pve-devel] [PATCH manager v7 01/14] api: /cluster/resources: add tags to returned properties Dominik Csapak
` (15 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
normal tags require 'VM.Config.Options' on the CT, admin tags require
'Sys.Modify' on '/'
a user can set/delete/reorder tags, as long as no admin tags get
added/removed
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PVE/LXC.pm | 31 +++++++++++++++++++++++++++++++
src/PVE/LXC/Config.pm | 3 ++-
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index fe63087..ce2f8ff 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1333,6 +1333,37 @@ sub check_ct_modify_config_perm {
} elsif ($opt eq 'hookscript') {
# For now this is restricted to root@pam
raise_perm_exc("changing the hookscript is only allowed for root\@pam");
+ } elsif ($opt eq 'tags') {
+ my $old_tags = {};
+ my $new_tags = {};
+ my $check_admin = 0;
+ my $check_user = 0;
+
+ map { $old_tags->{$_} = 1 } PVE::Tools::split_list($oldconf->{$opt});
+ map { $new_tags->{$_} = 1 } PVE::Tools::split_list($newconf->{$opt});
+
+ my $check_tags = sub {
+ my ($a, $b) = @_;
+ foreach my $tag (keys %$a) {
+ next if $b->{$tag};
+ if (PVE::JSONSchema::is_admin_pve_tag($tag)) {
+ $check_admin = 1;
+ } else {
+ $check_user = 1;
+ }
+ }
+ };
+
+ $check_tags->($old_tags, $new_tags);
+ $check_tags->($new_tags, $old_tags);
+
+ if ($check_admin) {
+ $rpcenv->check($authuser, "/", ['Sys.Modify']);
+ }
+
+ if ($check_user) {
+ $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
+ }
} else {
$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
}
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index b4b0261..2362d74 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -597,7 +597,8 @@ my $confdesc = {
},
tags => {
type => 'string', format => 'pve-tag-list',
- description => 'Tags of the Container. This is only meta information.',
+ description => 'Tags of the Container. This is only meta information. Prefixing a tag with'.
+ "'+' marks it as an admin tag and can only be set with 'Sys.Modify' on '/'.",
optional => 1,
},
debug => {
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 01/14] api: /cluster/resources: add tags to returned properties
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (9 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH container v7 1/1] check_ct_modify_config_perm: " Dominik Csapak
@ 2022-06-21 9:19 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 02/14] api: /version: add 'tag-style' Dominik Csapak
` (14 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:19 UTC (permalink / raw)
To: pve-devel
by querying 'lock' and 'tags' with 'get_guest_config_properties'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/API2/Cluster.pm | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 525a95a1..c1e96bd7 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -360,7 +360,8 @@ __PACKAGE__->register_method({
# we try to generate 'numbers' by using "$X + 0"
if (!$param->{type} || $param->{type} eq 'vm') {
- my $locked_vms = PVE::Cluster::get_guest_config_property('lock');
+ my $prop_list = [qw(lock tags)];
+ my $props = PVE::Cluster::get_guest_config_properties($prop_list);
for my $vmid (sort keys %$idlist) {
@@ -392,8 +393,10 @@ __PACKAGE__->register_method({
# only skip now to next to ensure that the pool stats above are filled, if eligible
next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1);
- if (defined(my $lock = $locked_vms->{$vmid}->{lock})) {
- $entry->{lock} = $lock;
+ for my $prop (@$prop_list) {
+ if (defined(my $value = $props->{$vmid}->{$prop})) {
+ $entry->{$prop} = $value;
+ }
}
if (defined($entry->{pool}) &&
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 01/14] api: /cluster/resources: add tags to returned properties
2022-06-21 9:19 ` [pve-devel] [PATCH manager v7 01/14] api: /cluster/resources: add tags to returned properties Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
0 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
one small nit inside
On 6/21/22 11:19, Dominik Csapak wrote:
> by querying 'lock' and 'tags' with 'get_guest_config_properties'
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> PVE/API2/Cluster.pm | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index 525a95a1..c1e96bd7 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -360,7 +360,8 @@ __PACKAGE__->register_method({
>
> # we try to generate 'numbers' by using "$X + 0"
> if (!$param->{type} || $param->{type} eq 'vm') {
> - my $locked_vms = PVE::Cluster::get_guest_config_property('lock');
> + my $prop_list = [qw(lock tags)];
> + my $props = PVE::Cluster::get_guest_config_properties($prop_list);
>
> for my $vmid (sort keys %$idlist) {
>
> @@ -392,8 +393,10 @@ __PACKAGE__->register_method({
> # only skip now to next to ensure that the pool stats above are filled, if eligible
> next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1);
>
> - if (defined(my $lock = $locked_vms->{$vmid}->{lock})) {
> - $entry->{lock} = $lock;
> + for my $prop (@$prop_list) {
consider @{$prop_list} instead of @$prop_list.
> + if (defined(my $value = $props->{$vmid}->{$prop})) {
> + $entry->{$prop} = $value;
> + }
> }
>
> if (defined($entry->{pool}) &&
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH manager v7 02/14] api: /version: add 'tag-style'
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (10 preceding siblings ...)
2022-06-21 9:19 ` [pve-devel] [PATCH manager v7 01/14] api: /cluster/resources: add tags to returned properties Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 03/14] ui: parse and save tag color overrides from /version Dominik Csapak
` (13 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
to be able to use it in the gui directly after login
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
PVE/API2.pm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/PVE/API2.pm b/PVE/API2.pm
index a4256160..26fcc02e 100644
--- a/PVE/API2.pm
+++ b/PVE/API2.pm
@@ -111,6 +111,11 @@ __PACKAGE__->register_method ({
optional => 1,
description => 'The default console viewer to use.',
},
+ 'tag-style' => {
+ type => 'string',
+ optional => 1,
+ description => 'Cluster wide tag style overrides',
+ },
},
},
code => sub {
@@ -119,7 +124,7 @@ __PACKAGE__->register_method ({
my $res = {};
my $datacenter_confg = eval { PVE::Cluster::cfs_read_file('datacenter.cfg') } // {};
- for my $k (qw(console)) {
+ for my $k (qw(console tag-style)) {
$res->{$k} = $datacenter_confg->{$k} if exists $datacenter_confg->{$k};
}
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 03/14] ui: parse and save tag color overrides from /version
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (11 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 02/14] api: /version: add 'tag-style' Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 04/14] ui: tree/ResourceTree: collect tags on update Dominik Csapak
` (12 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
into a global list of overrides. on update, also parse the values
from the browser localstore
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Utils.js | 40 +++++++++++++++++++++++++++++++++++++++
www/manager6/Workspace.js | 13 +++++++++++++
2 files changed, 53 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 7ca6a271..8653a4b9 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1803,6 +1803,46 @@ Ext.define('PVE.Utils', {
},
notesTemplateVars: ['cluster', 'guestname', 'node', 'vmid'],
+
+ parseTagOverrides: function(overrides) {
+ let colors = {};
+ (overrides || "").split(';').forEach(color => {
+ if (!color) {
+ return;
+ }
+ let [tag, color_hex, font_hex] = color.split(':');
+ let r = parseInt(color_hex.slice(0, 2), 16);
+ let g = parseInt(color_hex.slice(2, 4), 16);
+ let b = parseInt(color_hex.slice(4, 6), 16);
+ colors[tag] = [r, g, b];
+ if (font_hex) {
+ colors[tag].push(parseInt(font_hex.slice(0, 2), 16));
+ colors[tag].push(parseInt(font_hex.slice(2, 4), 16));
+ colors[tag].push(parseInt(font_hex.slice(4, 6), 16));
+ }
+ });
+ return colors;
+ },
+
+ tagOverrides: {},
+
+ updateTagOverrides: function(colors) {
+ let sp = Ext.state.Manager.getProvider();
+ let color_state = sp.get('colors', '');
+ let browser_colors = PVE.Utils.parseTagOverrides(color_state);
+ PVE.Utils.tagOverrides = Ext.apply({}, browser_colors, colors);
+ Ext.GlobalEvents.fireEvent('tag-color-override');
+ },
+
+ updateTagSettings: function(overrides, style) {
+ PVE.Utils.updateTagOverrides(PVE.Utils.parseTagOverrides(overrides ?? ""));
+
+ if (style === undefined || style === '__default__') {
+ style = 'circle';
+ }
+
+ Ext.ComponentQuery.query('pveResourceTree')[0].setUserCls(`proxmox-tags-${style}`);
+ },
},
singleton: true,
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 37d772b8..cd31249e 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -155,6 +155,7 @@ Ext.define('PVE.StdWorkspace', {
success: function(response) {
PVE.VersionInfo = response.result.data;
me.updateVersionInfo();
+ me.updateTags();
},
});
@@ -213,6 +214,18 @@ Ext.define('PVE.StdWorkspace', {
ui.updateLayout();
},
+ updateTags: function() {
+ let me = this;
+ let colors = PVE.VersionInfo?.['tag-style']?.colors;
+ let shape = PVE.VersionInfo?.['tag-style']?.['tree-shape'];
+
+ PVE.Utils.updateTagSettings(colors, shape);
+ if (colors) {
+ // refresh tree once
+ PVE.data.ResourceStore.fireEvent('load');
+ }
+ },
+
initComponent: function() {
let me = this;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 04/14] ui: tree/ResourceTree: collect tags on update
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (12 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 03/14] ui: parse and save tag color overrides from /version Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 05/14] ui: add form/TagColorGrid Dominik Csapak
` (11 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
into a global list, so that we have it avaiable anywhere
also add the tags from the tagOverrides on update into the list
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Utils.js | 7 +++++++
www/manager6/data/ResourceStore.js | 6 ++++++
www/manager6/tree/ResourceTree.js | 16 ++++++++++++++--
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 8653a4b9..5242e778 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1804,6 +1804,13 @@ Ext.define('PVE.Utils', {
notesTemplateVars: ['cluster', 'guestname', 'node', 'vmid'],
+ tagList: new Set(),
+
+ updateTagList: function(tags) {
+ let override_tags = Object.keys(PVE.Utils.tagOverrides);
+ PVE.Utils.tagList = [...new Set([...tags, ...override_tags])].sort();
+ },
+
parseTagOverrides: function(overrides) {
let colors = {};
(overrides || "").split(';').forEach(color => {
diff --git a/www/manager6/data/ResourceStore.js b/www/manager6/data/ResourceStore.js
index c7b72306..b18f7dd8 100644
--- a/www/manager6/data/ResourceStore.js
+++ b/www/manager6/data/ResourceStore.js
@@ -293,6 +293,12 @@ Ext.define('PVE.data.ResourceStore', {
sortable: true,
width: 100,
},
+ tags: {
+ header: gettext('Tags'),
+ type: 'string',
+ hidden: true,
+ sortable: true,
+ },
};
let fields = [];
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index be90d4f7..139defab 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -226,6 +226,10 @@ Ext.define('PVE.tree.ResourceTree', {
let stateid = 'rid';
+ const changedFields = [
+ 'text', 'running', 'template', 'status', 'qmpstatus', 'hastate', 'lock', 'tags',
+ ];
+
let updateTree = function() {
store.suspendEvents();
@@ -261,7 +265,7 @@ Ext.define('PVE.tree.ResourceTree', {
}
// tree item has been updated
- for (const field of ['text', 'running', 'template', 'status', 'qmpstatus', 'hastate', 'lock']) {
+ for (const field of changedFields) {
if (item.data[field] !== olditem.data[field]) {
changed = true;
break;
@@ -294,7 +298,14 @@ Ext.define('PVE.tree.ResourceTree', {
}
}
- rstore.each(function(item) { // add new items
+ let tags = new Set();
+
+ rstore.each(function(item) { // add new items and collect tags
+ if (item.data.tags) {
+ item.data.tags.split(/[,; ]/).filter(t => !!t).forEach((tag) => {
+ tags.add(tag);
+ });
+ }
let olditem = index[item.data.id];
if (olditem) {
return;
@@ -310,6 +321,7 @@ Ext.define('PVE.tree.ResourceTree', {
}
});
+ PVE.Utils.updateTagList(tags);
store.resumeEvents();
store.fireEvent('refresh', store);
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 05/14] ui: add form/TagColorGrid
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (13 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 04/14] ui: tree/ResourceTree: collect tags on update Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 06/14] ui: dc/OptionView: add editors for tag settings Dominik Csapak
` (10 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
this provides a basic grid to edit a list of tag color overrides.
We'll use this for editing the datacenter.cfg overrides and the
browser storage overrides.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/css/ext6-pve.css | 5 +
www/manager6/Makefile | 1 +
www/manager6/Utils.js | 2 +
www/manager6/form/TagColorGrid.js | 357 ++++++++++++++++++++++++++++++
4 files changed, 365 insertions(+)
create mode 100644 www/manager6/form/TagColorGrid.js
diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index dadb84a9..f7d0c420 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -651,3 +651,8 @@ table.osds td:first-of-type {
background-color: rgb(245, 245, 245);
color: #000;
}
+
+.x-pveColorPicker-default-cell > .x-grid-cell-inner {
+ padding-top: 0px;
+ padding-bottom: 0px;
+}
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d16770b1..60ae421e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -73,6 +73,7 @@ JSSRC= \
form/VNCKeyboardSelector.js \
form/ViewSelector.js \
form/iScsiProviderSelector.js \
+ form/TagColorGrid.js \
grid/BackupView.js \
grid/FirewallAliases.js \
grid/FirewallOptions.js \
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 5242e778..bd0490a4 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1850,6 +1850,8 @@ Ext.define('PVE.Utils', {
Ext.ComponentQuery.query('pveResourceTree')[0].setUserCls(`proxmox-tags-${style}`);
},
+
+ tagCharRegex: /^[a-z0-9+_.-]$/i,
},
singleton: true,
diff --git a/www/manager6/form/TagColorGrid.js b/www/manager6/form/TagColorGrid.js
new file mode 100644
index 00000000..3ad8e07f
--- /dev/null
+++ b/www/manager6/form/TagColorGrid.js
@@ -0,0 +1,357 @@
+Ext.define('PVE.form.ColorPicker', {
+ extend: 'Ext.form.FieldContainer',
+ alias: 'widget.pveColorPicker',
+
+ defaultBindProperty: 'value',
+
+ config: {
+ value: null,
+ },
+
+ height: 24,
+
+ layout: {
+ type: 'hbox',
+ align: 'stretch',
+ },
+
+ getValue: function() {
+ return this.realvalue.slice(1);
+ },
+
+ setValue: function(value) {
+ let me = this;
+ me.setColor(value);
+ if (value && value.length === 6) {
+ me.picker.value = value[0] !== '#' ? `#${value}` : value;
+ }
+ },
+
+ setColor: function(value) {
+ let me = this;
+ let oldValue = me.realvalue;
+ me.realvalue = value;
+ let color = value.length === 6 ? `#${value}` : undefined;
+ me.down('#picker').setStyle('background-color', color);
+ me.down('#text').setValue(value ?? "");
+ me.fireEvent('change', me, me.realvalue, oldValue);
+ },
+
+ initComponent: function() {
+ let me = this;
+ me.picker = document.createElement('input');
+ me.picker.type = 'color';
+ me.picker.style = `opacity: 0; border: 0px; width: 100%; height: ${me.height}px`;
+ me.picker.value = `${me.value}`;
+
+ me.items = [
+ {
+ xtype: 'textfield',
+ itemId: 'text',
+ minLength: !me.allowBlank ? 6 : undefined,
+ maxLength: 6,
+ enforceMaxLength: true,
+ allowBlank: me.allowBlank,
+ emptyText: me.allowBlank ? gettext('Automatic') : undefined,
+ maskRe: /[a-f0-9]/i,
+ regex: /^[a-f0-9]{6}$/i,
+ flex: 1,
+ listeners: {
+ change: function(field, value) {
+ me.setValue(value);
+ },
+ },
+ },
+ {
+ xtype: 'box',
+ style: {
+ 'margin-left': '1px',
+ border: '1px solid #cfcfcf',
+ },
+ itemId: 'picker',
+ width: 24,
+ contentEl: me.picker,
+ },
+ ];
+
+ me.callParent();
+ me.picker.oninput = function() {
+ me.setColor(me.picker.value.slice(1));
+ };
+ },
+});
+
+Ext.define('PVE.form.TagColorGrid', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pveTagColorGrid',
+
+ mixins: [
+ 'Ext.form.field.Field',
+ ],
+
+ allowBlank: true,
+ selectAll: false,
+ isFormField: true,
+ deleteEmpty: false,
+ selModel: 'checkboxmodel',
+
+ config: {
+ deleteEmpty: false,
+ },
+
+ emptyText: gettext('No Overrides'),
+ viewConfig: {
+ deferEmptyText: false,
+ },
+
+ setValue: function(value) {
+ let me = this;
+ let colors;
+ if (Ext.isObject(value)) {
+ colors = value.colors;
+ } else {
+ colors = value;
+ }
+ if (!colors) {
+ me.getStore().removeAll();
+ me.checkChange();
+ return me;
+ }
+ let entries = (colors.split(';') || []).map((entry) => {
+ let [tag, bg, fg] = entry.split(':');
+ fg = fg || "";
+ return {
+ tag,
+ color: bg,
+ text: fg,
+ };
+ });
+ me.getStore().setData(entries);
+ me.checkChange();
+ return me;
+ },
+
+ getValue: function() {
+ let me = this;
+ let values = [];
+ me.getStore().each((rec) => {
+ if (rec.data.tag) {
+ let val = `${rec.data.tag}:${rec.data.color}`;
+ if (rec.data.text) {
+ val += `:${rec.data.text}`;
+ }
+ values.push(val);
+ }
+ });
+ return values.join(';');
+ },
+
+ getErrors: function(value) {
+ let me = this;
+ let emptyTag = false;
+ let notValidColor = false;
+ let colorRegex = new RegExp(/^[0-9a-f]{6}$/i);
+ me.getStore().each((rec) => {
+ if (!rec.data.tag) {
+ emptyTag = true;
+ }
+ if (!rec.data.color?.match(colorRegex)) {
+ notValidColor = true;
+ }
+ if (rec.data.text && !rec.data.text?.match(colorRegex)) {
+ notValidColor = true;
+ }
+ });
+ let errors = [];
+ if (emptyTag) {
+ errors.push(gettext('Tag must not be empty.'));
+ }
+ if (notValidColor) {
+ errors.push(gettext('Not a valid color.'));
+ }
+ return errors;
+ },
+
+ // override framework function to implement deleteEmpty behaviour
+ getSubmitData: function() {
+ let me = this,
+ data = null,
+ val;
+ if (!me.disabled && me.submitValue) {
+ val = me.getValue();
+ if (val !== null && val !== '') {
+ data = {};
+ data[me.getName()] = val;
+ } else if (me.getDeleteEmpty()) {
+ data = {};
+ data.delete = me.getName();
+ }
+ }
+ return data;
+ },
+
+
+ controller: {
+ xclass: 'Ext.app.ViewController',
+
+ addLine: function() {
+ let me = this;
+ me.getView().getStore().add({
+ tag: '',
+ color: '',
+ text: '',
+ });
+ },
+
+ removeSelection: function() {
+ let me = this;
+ let view = me.getView();
+ let selection = view.getSelection();
+ if (selection === undefined) {
+ return;
+ }
+
+ selection.forEach((sel) => {
+ view.getStore().remove(sel);
+ });
+ view.checkChange();
+ },
+
+ tagChange: function(field, newValue, oldValue) {
+ let me = this;
+ let rec = field.getWidgetRecord();
+ if (!rec) {
+ return;
+ }
+ if (newValue && newValue !== oldValue) {
+ let newrgb = Proxmox.Utils.stringToRGB(newValue);
+ let newvalue = Proxmox.Utils.rgbToHex(newrgb);
+ if (!rec.get('color')) {
+ rec.set('color', newvalue);
+ } else if (oldValue) {
+ let oldrgb = Proxmox.Utils.stringToRGB(oldValue);
+ let oldvalue = Proxmox.Utils.rgbToHex(oldrgb);
+ if (rec.get('color') === oldvalue) {
+ rec.set('color', newvalue);
+ }
+ }
+ }
+ me.fieldChange(field, newValue, oldValue);
+ },
+
+ backgroundChange: function(field, newValue, oldValue) {
+ let me = this;
+ let rec = field.getWidgetRecord();
+ if (!rec) {
+ return;
+ }
+ if (newValue && newValue !== oldValue) {
+ let newrgb = Proxmox.Utils.hexToRGB(newValue);
+ let newcls = Proxmox.Utils.getTextContrastClass(newrgb);
+ let hexvalue = newcls === 'dark' ? '000000' : 'FFFFFF';
+ if (!rec.get('text')) {
+ rec.set('text', hexvalue);
+ } else if (oldValue) {
+ let oldrgb = Proxmox.Utils.hexToRGB(oldValue);
+ let oldcls = Proxmox.Utils.getTextContrastClass(oldrgb);
+ let oldvalue = oldcls === 'dark' ? '000000' : 'FFFFFF';
+ if (rec.get('text') === oldvalue) {
+ rec.set('text', hexvalue);
+ }
+ }
+ }
+ me.fieldChange(field, newValue, oldValue);
+ },
+
+ fieldChange: function(field, newValue, oldValue) {
+ let me = this;
+ let view = me.getView();
+ let rec = field.getWidgetRecord();
+ if (!rec) {
+ return;
+ }
+ let column = field.getWidgetColumn();
+ rec.set(column.dataIndex, newValue);
+ view.checkChange();
+ },
+ },
+
+ tbar: [
+ {
+ text: gettext('Add'),
+ handler: 'addLine',
+ },
+ {
+ xtype: 'proxmoxButton',
+ text: gettext('Remove'),
+ handler: 'removeSelection',
+ disabled: true,
+ },
+ ],
+
+ columns: [
+ {
+ header: 'Tag',
+ dataIndex: 'tag',
+ xtype: 'widgetcolumn',
+ onWidgetAttach: function(col, widget, rec) {
+ widget.getStore().setData(PVE.Utils.tagList.map(v => ({ tag: v })));
+ },
+ widget: {
+ xtype: 'combobox',
+ isFormField: false,
+ maskRe: PVE.Utils.tagCharRegex,
+ allowBlank: false,
+ queryMode: 'local',
+ displayField: 'tag',
+ valueField: 'tag',
+ store: {},
+ listeners: {
+ change: 'tagChange',
+ },
+ },
+ flex: 1,
+ },
+ {
+ header: gettext('Background'),
+ xtype: 'widgetcolumn',
+ flex: 1,
+ dataIndex: 'color',
+ widget: {
+ xtype: 'pveColorPicker',
+ isFormField: false,
+ listeners: {
+ change: 'backgroundChange',
+ },
+ },
+ },
+ {
+ header: gettext('Text'),
+ xtype: 'widgetcolumn',
+ flex: 1,
+ dataIndex: 'text',
+ widget: {
+ xtype: 'pveColorPicker',
+ allowBlank: true,
+ isFormField: false,
+ listeners: {
+ change: 'fieldChange',
+ },
+ },
+ },
+ ],
+
+ store: {
+ listeners: {
+ update: function() {
+ this.commitChanges();
+ },
+ },
+ },
+
+ initComponent: function() {
+ let me = this;
+ me.callParent();
+ me.initField();
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 05/14] ui: add form/TagColorGrid
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 05/14] ui: add form/TagColorGrid Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
0 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
One thing I noticed in a few places regarding variable names was that they
probably could use some camel casing for better readabilty. E.g.:
newcls -> newCls
newrbd -> newRgb or maybe newRGB?
More comments inline.
On 6/21/22 11:20, Dominik Csapak wrote:
> this provides a basic grid to edit a list of tag color overrides.
> We'll use this for editing the datacenter.cfg overrides and the
> browser storage overrides.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[...]
> +++ b/www/manager6/form/TagColorGrid.js
> @@ -0,0 +1,357 @@
> +Ext.define('PVE.form.ColorPicker', {
> + extend: 'Ext.form.FieldContainer',
> + alias: 'widget.pveColorPicker',
> +
> + defaultBindProperty: 'value',
> +
> + config: {
> + value: null,
> + },
> +
> + height: 24,
> +
> + layout: {
> + type: 'hbox',
> + align: 'stretch',
> + },
> +
> + getValue: function() {
> + return this.realvalue.slice(1);
> + },
> +
> + setValue: function(value) {
> + let me = this;
> + me.setColor(value);
> + if (value && value.length === 6) {
> + me.picker.value = value[0] !== '#' ? `#${value}` : value;
if we do check the length to be 6 characters long, can we run into the situation
that it is already prefixed by a #?
> + }
> + },
> +
> + setColor: function(value) {
> + let me = this;
> + let oldValue = me.realvalue;
> + me.realvalue = value;
> + let color = value.length === 6 ? `#${value}` : undefined;
> + me.down('#picker').setStyle('background-color', color);
> + me.down('#text').setValue(value ?? "");
> + me.fireEvent('change', me, me.realvalue, oldValue);
> + },
[...]
> +
> + backgroundChange: function(field, newValue, oldValue) {
> + let me = this;
> + let rec = field.getWidgetRecord();
> + if (!rec) {
> + return;
> + }
> + if (newValue && newValue !== oldValue) {
> + let newrgb = Proxmox.Utils.hexToRGB(newValue);
> + let newcls = Proxmox.Utils.getTextContrastClass(newrgb);
> + let hexvalue = newcls === 'dark' ? '000000' : 'FFFFFF';
s/hexvalue/textHexVal/ or something in that direction so the variable name
reflects what is contains?
> + if (!rec.get('text')) {
> + rec.set('text', hexvalue);
> + } else if (oldValue) {
> + let oldrgb = Proxmox.Utils.hexToRGB(oldValue);
> + let oldcls = Proxmox.Utils.getTextContrastClass(oldrgb);
> + let oldvalue = oldcls === 'dark' ? '000000' : 'FFFFFF';
> + if (rec.get('text') === oldvalue) {
> + rec.set('text', hexvalue);
> + }
> + }
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH manager v7 06/14] ui: dc/OptionView: add editors for tag settings
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (14 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 05/14] ui: add form/TagColorGrid Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag Dominik Csapak
` (9 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
namely for 'tag-tree-style' and 'tag-colors'.
display the tag overrides directly as they will appear as tags
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Utils.js | 20 +++++++++
www/manager6/dc/OptionView.js | 84 +++++++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index bd0490a4..4bd8875b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1851,6 +1851,26 @@ Ext.define('PVE.Utils', {
Ext.ComponentQuery.query('pveResourceTree')[0].setUserCls(`proxmox-tags-${style}`);
},
+ tagTreeStyles: {
+ '__default__': Proxmox.Utils.defaultText,
+ 'full': gettext('Full'),
+ 'circle': gettext('Circle'),
+ 'dense': gettext('Dense'),
+ 'none': Proxmox.Utils.NoneText,
+ },
+
+ renderTags: function(tagstext, overrides) {
+ let text = '';
+ if (tagstext) {
+ let tags = (tagstext.split(/[,; ]/) || []).filter(t => !!t);
+ text += ' ';
+ tags.forEach((tag) => {
+ text += Proxmox.Utils.getTagElement(tag, overrides);
+ });
+ }
+ return text;
+ },
+
tagCharRegex: /^[a-z0-9+_.-]$/i,
},
diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index 51696982..6dbe1d28 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -5,6 +5,7 @@ Ext.define('PVE.dc.OptionView', {
onlineHelp: 'datacenter_configuration_file',
monStoreErrors: true,
+ userCls: 'proxmox-tags-full',
add_inputpanel_row: function(name, text, opts) {
var me = this;
@@ -305,6 +306,86 @@ Ext.define('PVE.dc.OptionView', {
submitValue: true,
}],
});
+ me.rows['tag-style'] = {
+ required: true,
+ renderer: (value) => {
+ if (value === undefined) {
+ return gettext('No Overrides');
+ }
+ let colors = PVE.Utils.parseTagOverrides(value.colors);
+ let shape = value['tree-shape'];
+ let shapeText = PVE.Utils.tagTreeStyles[shape] ?? Proxmox.Utils.defaultText;
+ let txt = Ext.String.format(gettext("Tree Shape: {0}"), shapeText);
+ if (Object.keys(colors).length > 0) {
+ txt += ', ';
+ }
+ for (const tag of Object.keys(colors)) {
+ txt += Proxmox.Utils.getTagElement(tag, colors);
+ }
+ return txt;
+ },
+ header: gettext('Tag Style Override'),
+ editor: {
+ xtype: 'proxmoxWindowEdit',
+ width: 800,
+ subject: gettext('Tag Color Override'),
+ fieldDefaults: {
+ labelWidth: 100,
+ },
+ url: '/api2/extjs/cluster/options',
+ items: [
+ {
+ xtype: 'inputpanel',
+ setValues: function(values) {
+ if (values === undefined) {
+ return undefined;
+ }
+ values = values?.['tag-style'] ?? {};
+ values['tree-shape'] = values?.['tree-shape'] || '__default__';
+ return Proxmox.panel.InputPanel.prototype.setValues.call(this, values);
+ },
+ onGetValues: function(values) {
+ let style = {};
+ if (values.colors) {
+ style.colors = values.colors;
+ }
+ if (values['tree-shape']) {
+ style['tree-shape'] = values['tree-shape'];
+ }
+ let value = PVE.Parser.printPropertyString(style);
+ if (value === '') {
+ return {
+ delete: 'tag-style',
+ };
+ }
+ return {
+ 'tag-style': value,
+ };
+ },
+ items: [
+ {
+ name: 'tree-shape',
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Tree Shape'),
+ comboItems: Object.entries(PVE.Utils.tagTreeStyles),
+ defaultValue: '__default__',
+ deleteEmpty: true,
+ },
+ {
+ xtype: 'displayfield',
+ fieldLabel: gettext('Color Overrides'),
+ },
+ {
+ name: 'colors',
+ xtype: 'pveTagColorGrid',
+ deleteEmpty: true,
+ height: 300,
+ },
+ ],
+ },
+ ],
+ },
+ };
me.selModel = Ext.create('Ext.selection.RowModel', {});
@@ -340,6 +421,9 @@ Ext.define('PVE.dc.OptionView', {
if (rec.data.value === '__default__') {
delete PVE.VersionInfo.console;
}
+
+ let tagStyle = store.getById('tag-style')?.data?.value;
+ PVE.Utils.updateTagSettings(tagStyle?.colors, tagStyle?.['tree-shape']);
});
me.on('activate', me.rstore.startUpdate);
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (15 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 06/14] ui: dc/OptionView: add editors for tag settings Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 08/14] ui: add form/TagEdit.js Dominik Csapak
` (8 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
displays a single tag, with the ability to edit inline on click (when
the mode is set to editable). This brings up a list of globally available tags
for simple selection.
Also has a mode for adding a new Tag.
This has a 'layoutCallback' which will be called on input, so that the parent
component can update the layout when the content changes.
This is necessary since we circumvent the extjs logic for updating.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/form/Tag.js | 276 +++++++++++++++++++++++++++++++++++++++
2 files changed, 277 insertions(+)
create mode 100644 www/manager6/form/Tag.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 60ae421e..9d610f71 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -74,6 +74,7 @@ JSSRC= \
form/ViewSelector.js \
form/iScsiProviderSelector.js \
form/TagColorGrid.js \
+ form/Tag.js \
grid/BackupView.js \
grid/FirewallAliases.js \
grid/FirewallOptions.js \
diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
new file mode 100644
index 00000000..0a754edb
--- /dev/null
+++ b/www/manager6/form/Tag.js
@@ -0,0 +1,276 @@
+Ext.define('Proxmox.Tag', {
+ extend: 'Ext.Component',
+ alias: 'widget.pmxTag',
+
+ // if set to true, displays 'Add Tag' and a plus symbol
+ addTag: false,
+
+ // callback to update the layout in the containing element
+ // this is necessary since we circumvent extjs layout with 'contentEditable'
+ layoutCallback: Ext.emptyFn,
+
+ style: {
+ 'white-space': 'nowrap',
+ },
+
+ icons: {
+ addTag: 'plus',
+ editable: 'minus',
+ normal: '',
+ edit: 'check',
+ },
+
+ faIconStyle: '-square',
+
+ mode: 'normal',
+
+ // we need to do this in mousedown, because that triggers before
+ // focusleave (which triggers before click)
+ onMouseDown: function(event) {
+ let me = this;
+ if (event.target.tagName !== 'I') {
+ return;
+ }
+ switch (me.mode) {
+ case 'editable':
+ if (me.addTag) {
+ break;
+ }
+ me.setText('');
+ me.finishEdit();
+ break;
+ case 'edit':
+ if (me.addTag && me.tagEl().innerHTML === '') {
+ event.browserEvent.preventDefault();
+ event.browserEvent.stopPropagation();
+ break;
+ }
+ me.finishEdit();
+ break;
+ default: break;
+ }
+ },
+
+ onClick: function(event) {
+ let me = this;
+ if (event.target.tagName !== 'SPAN' && !me.addTag) {
+ return;
+ }
+ if (me.mode === 'editable') {
+ me.startEdit();
+ }
+ },
+
+ startEdit: function() {
+ let me = this;
+ me.setMode('edit');
+
+ // select text in the element
+ let range = document.createRange();
+ range.selectNodeContents(me.tagEl());
+ let sel = window.getSelection();
+ sel.removeAllRanges();
+ sel.addRange(range);
+
+ me.showPicker();
+ },
+
+ showPicker: function() {
+ let me = this;
+ if (!me.picker) {
+ me.picker = Ext.widget({
+ xtype: 'boundlist',
+ minWidth: 70,
+ scrollable: true,
+ floating: true,
+ hidden: true,
+ userCls: 'proxmox-tags-full',
+ displayField: 'tag',
+ itemTpl: [
+ '{[Proxmox.Utils.getTagElement(values.tag, PVE.Utils.tagOverrides)]}',
+ ],
+ store: [],
+ listeners: {
+ select: function(picker, rec) {
+ me.setText(rec.data.tag);
+ me.finishEdit();
+ },
+ },
+ });
+ }
+ me.picker.getStore().clearFilter();
+ let taglist = PVE.Utils.tagList.map(v => ({ tag: v }));
+ if (taglist.length < 1) {
+ return;
+ }
+ me.picker.getStore().setData(taglist);
+ me.picker.showBy(me, 'tl-bl');
+ me.picker.setMaxHeight(200);
+ },
+
+ finishEdit: function(update = true) {
+ let me = this;
+ me.picker?.hide();
+
+ let tag = me.tagEl().innerHTML;
+ if (!me.addTag) {
+ if (tag === '') {
+ // "delete" ourselves
+ me.setVisible(false);
+ }
+ me.tag = tag;
+ me.updateColor(me.tag);
+ }
+
+ if (update) {
+ me.fireEvent('change', tag);
+ }
+
+ me.tagEl().contentEditable = false;
+ me.setMode('editable');
+ },
+
+ cancelEdit: function(list, event) {
+ let me = this;
+ if (me.mode === 'edit') {
+ me.setText(me.tag);
+ me.finishEdit(false);
+ }
+ },
+
+ setText: function(text) {
+ let me = this;
+ me.tagEl().innerHTML = text;
+ me.layoutCallback();
+ },
+
+ getTag: function() {
+ return this.tagEl().innerHTML;
+ },
+
+ setMode: function(mode) {
+ let me = this;
+ let icon = me.icons[me.addTag ? 'addTag' : mode];
+ let iconStyle = 'cursor: pointer; padding-right: 6px;';
+ let text = me.tag;
+ let cursor = 'pointer';
+ let padding = '0px';
+ switch (mode) {
+ case 'normal':
+ iconStyle += 'display: none;';
+ padding = undefined;
+ break;
+ case 'editable':
+ break;
+ case 'edit':
+ me.tagEl().contentEditable = true;
+ text = '';
+ cursor = undefined;
+ break;
+ default: return;
+ }
+
+ if (me.addTag) {
+ me.setText(text);
+ me.setStyle('cursor', cursor);
+ }
+
+ me.setStyle('padding-right', padding);
+
+ me.iconEl().classList = `fa fa-${icon}${me.faIconStyle}`;
+ me.iconEl().style = iconStyle;
+ me.mode = mode;
+ },
+
+ onKeyPress: function(event) {
+ let me = this;
+ let key = event.browserEvent.key;
+ if (key === "Enter") {
+ if (!me.addTag || me.tagEl().innerHTML !== '') {
+ me.finishEdit();
+ return;
+ }
+ } else if (key.match(PVE.Utils.tagCharRegex)) {
+ return;
+ }
+ event.browserEvent.preventDefault();
+ event.browserEvent.stopPropagation();
+ },
+
+ onInput: function() {
+ let me = this;
+ let tag = me.tagEl().innerHTML;
+ me.layoutCallback();
+ me.picker.getStore().filter({
+ property: 'tag',
+ value: tag,
+ });
+ },
+
+ listeners: {
+ mousedown: 'onMouseDown',
+ click: 'onClick',
+ focusleave: 'cancelEdit',
+ keypress: 'onKeyPress',
+ input: 'onInput',
+ element: 'el',
+ },
+
+ updateColor: function(tag) {
+ let me = this;
+ let rgb = PVE.Utils.tagOverrides[tag] ?? Proxmox.Utils.stringToRGB(tag);
+
+ let cls = Proxmox.Utils.getTextContrastClass(rgb);
+ let color = Proxmox.Utils.rgbToCss(rgb);
+ me.setUserCls(`proxmox-tag-${cls}`);
+ me.setStyle('background-color', color);
+ if (rgb.length > 3) {
+ let fgcolor = Proxmox.Utils.rgbToCss([rgb[3], rgb[4], rgb[5]]);
+ me.setStyle('color', fgcolor);
+ } else {
+ me.setStyle('color');
+ }
+ },
+
+ tagEl: function() {
+ return this.el?.dom?.children?.[0];
+ },
+
+ iconEl: function() {
+ return this.el?.dom?.children?.[1];
+ },
+
+ initComponent: function() {
+ let me = this;
+ if (me.tag === undefined && !me.addTag) {
+ throw "no tag given";
+ }
+ me.mode = me.mode ?? 'normal';
+
+ if (me.addTag) {
+ me.tag = gettext('Add Tag');
+ me.mode = 'editable';
+ me.setUserCls(`proxmox-tag-dark`);
+ }
+
+ let iconStyle = me.mode !== 'editable' ? 'display: none' : 'padding-right: 6px;';
+ let iconCls = me.icons[me.addTag ? 'addTag' : me.mode];
+
+ let icon = ` <i style="cursor: pointer; ${iconStyle}" class="fa fa-${iconCls}${me.faIconStyle}"></i>`;
+ me.html = `<span style="padding-right: 1px">${me.tag}</span>${icon}`;
+
+ me.callParent();
+ if (me.addTag) {
+ me.setStyle('cursor', 'pointer');
+ } else {
+ me.updateColor(me.tag);
+ }
+ me.setStyle('padding-right', me.mode === 'editable' ? '0px' : undefined);
+
+ if (!me.addTag) {
+ me.mon(Ext.GlobalEvents, 'tag-color-override', function() {
+ me.updateColor(me.tag);
+ });
+ }
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
2022-09-14 14:36 ` Aaron Lauterer
0 siblings, 1 reply; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
This component seems to not be working as expected.
In Firefox (105.0) I get all kinds of glitches and investigating it a bit, it
seems like the HTML & CSS classes manipulation doesn't seem to work correctly.
For example, if I edit an existing tag and select another existing one from the
appearing drop down menu, the tag has a different set of HTML tags than before.
The "Add tag" button also seems somewhat broken in that regard, e.g. showing the
plus icon more than once.
On Chromium (105.0.5195.102), I noticed, that I cannot select a pre-existing tag
when adding a new or editing an existing tag, but the tags themselves look okay.
I would also recommend that we do test this in Safari and Edge (the officially
supported browser).
On 6/21/22 11:20, Dominik Csapak wrote:
> displays a single tag, with the ability to edit inline on click (when
> the mode is set to editable). This brings up a list of globally available tags
> for simple selection.
>
> Also has a mode for adding a new Tag.
>
> This has a 'layoutCallback' which will be called on input, so that the parent
> component can update the layout when the content changes.
> This is necessary since we circumvent the extjs logic for updating.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/form/Tag.js | 276 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 277 insertions(+)
> create mode 100644 www/manager6/form/Tag.js
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag
2022-09-14 14:15 ` Aaron Lauterer
@ 2022-09-14 14:36 ` Aaron Lauterer
0 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:36 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
I have to follow up here as I noticed that in a private Firefox window it did
work much better and as expected. I suspect some addon is interfereing here.
Which one I cannot say yet.
On 9/14/22 16:15, Aaron Lauterer wrote:
> This component seems to not be working as expected.
>
> In Firefox (105.0) I get all kinds of glitches and investigating it a bit, it
> seems like the HTML & CSS classes manipulation doesn't seem to work correctly.
> For example, if I edit an existing tag and select another existing one from the
> appearing drop down menu, the tag has a different set of HTML tags than before.
>
> The "Add tag" button also seems somewhat broken in that regard, e.g. showing the
> plus icon more than once.
>
>
> On Chromium (105.0.5195.102), I noticed, that I cannot select a pre-existing tag
> when adding a new or editing an existing tag, but the tags themselves look okay.
>
> I would also recommend that we do test this in Safari and Edge (the officially
> supported browser).
>
>
> On 6/21/22 11:20, Dominik Csapak wrote:
>> displays a single tag, with the ability to edit inline on click (when
>> the mode is set to editable). This brings up a list of globally available tags
>> for simple selection.
>>
>> Also has a mode for adding a new Tag.
>>
>> This has a 'layoutCallback' which will be called on input, so that the parent
>> component can update the layout when the content changes.
>> This is necessary since we circumvent the extjs logic for updating.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> www/manager6/Makefile | 1 +
>> www/manager6/form/Tag.js | 276 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 277 insertions(+)
>> create mode 100644 www/manager6/form/Tag.js
>>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH manager v7 08/14] ui: add form/TagEdit.js
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (16 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 07/14] ui: add form/Tag Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 09/14] ui: {lxc, qemu}/Config: show Tags and make them editable Dominik Csapak
` (7 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
this is a wrapper container for holding a list of (editable) tags
intended to be used in the lxc/qemu status toolbar
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/form/TagEdit.js | 151 +++++++++++++++++++++++++++++++++++
2 files changed, 152 insertions(+)
create mode 100644 www/manager6/form/TagEdit.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 9d610f71..eb4be4c5 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -75,6 +75,7 @@ JSSRC= \
form/iScsiProviderSelector.js \
form/TagColorGrid.js \
form/Tag.js \
+ form/TagEdit.js \
grid/BackupView.js \
grid/FirewallAliases.js \
grid/FirewallOptions.js \
diff --git a/www/manager6/form/TagEdit.js b/www/manager6/form/TagEdit.js
new file mode 100644
index 00000000..5a267169
--- /dev/null
+++ b/www/manager6/form/TagEdit.js
@@ -0,0 +1,151 @@
+Ext.define('PVE.panel.TagEditContainer', {
+ extend: 'Ext.container.Container',
+ alias: 'widget.pveTagEditContainer',
+
+ tagCount: 0,
+
+ layout: {
+ type: 'hbox',
+ align: 'stretch',
+ },
+
+ loadTags: function(tagstring = '', inEdit, force = false) {
+ let me = this;
+
+ if (me.oldTags === tagstring && !force) {
+ return;
+ }
+
+ let tags = tagstring.split(/[;, ]/).filter((t) => !!t) || [];
+ me.suspendLayout = true;
+ me.tags = {};
+ me.removeAllTags();
+ tags.forEach((tag) => {
+ me.addTag(tag, inEdit);
+ });
+ me.suspendLayout = false;
+ me.updateLayout();
+ if (!force) {
+ me.oldTags = tagstring;
+ }
+ },
+
+ getEditBtnHtml: function() {
+ let me = this;
+ let cls = '';
+ let qtip = '';
+ if (me.editMode) {
+ qtip = gettext('Apply Changes');
+ cls = 'check';
+ } else {
+ qtip = gettext('Edit Tags');
+ cls = 'pencil';
+ }
+ return ` <i data-qtip="${qtip}" class="fa fa-${cls}"></i>`;
+ },
+
+ toggleEdit: function(cancel) {
+ let me = this;
+ me.editMode = !me.editMode;
+ let tagCount = 0;
+ me.tagFields.forEach((tag) => {
+ tag.setMode(me.editMode ? 'editable' : 'normal');
+ if (tag.isVisible() && !tag.addTag) {
+ tagCount++;
+ }
+ });
+
+ me.addTagBtn.setVisible(me.editMode);
+ me.editBtn.setHtml(me.getEditBtnHtml());
+ me.noTagsField.setVisible(!me.editMode && tagCount === 0);
+ me.cancelBtn.setVisible(me.editMode);
+
+ if (!me.editMode) {
+ let tags = [];
+ if (cancel) {
+ me.loadTags(me.oldTags, false, true);
+ } else {
+ me.tagFields.forEach((tag) => {
+ let tagValue = tag.getTag();
+ if (tag.isVisible() && tagValue) {
+ tags.push(tagValue);
+ }
+ });
+ tags = tags.join(',');
+ if (me.oldTags !== tags) {
+ me.oldTags = tags;
+ me.fireEvent('change', tags);
+ }
+ }
+ }
+ me.updateLayout();
+ },
+
+ removeAllTags: function() {
+ let me = this;
+ me.tagFields.forEach((tag) => {
+ me.remove(tag);
+ });
+ me.tagFields = [];
+ me.noTagsField.setVisible(true);
+ },
+
+ addTag: function(tag, inEdit) {
+ let me = this;
+ let tagField = me.insert(me.tagFields.length + 1, {
+ xtype: 'pmxTag',
+ tag,
+ mode: inEdit ? 'editable' : 'normal',
+ layoutCallback: () => me.updateLayout(),
+ });
+ me.tagFields.push(tagField);
+ me.noTagsField.setVisible(false);
+ },
+
+ initComponent: function() {
+ let me = this;
+ me.tagFields = [];
+ me.callParent();
+ me.noTagsField = me.add({
+ xtype: 'box',
+ html: gettext('No Tags'),
+ });
+ me.addTagBtn = me.add({
+ xtype: 'pmxTag',
+ addTag: true,
+ hidden: true,
+ layoutCallback: () => me.updateLayout(),
+ listeners: {
+ change: function(tag) {
+ me.addTag(tag, true);
+ },
+ },
+ });
+ me.cancelBtn = me.add({
+ xtype: 'box',
+ html: ` <i qtip-data="${gettext('Cancel')}" class="fa fa-times"></i>`,
+ style: {
+ cursor: 'pointer',
+ },
+ hidden: true,
+ listeners: {
+ click: () => me.toggleEdit(true),
+ element: 'el',
+ },
+ });
+ me.editBtn = me.add({
+ xtype: 'box',
+ html: me.getEditBtnHtml(),
+ style: {
+ cursor: 'pointer',
+ },
+ listeners: {
+ click: () => me.toggleEdit(false),
+ element: 'el',
+ },
+ });
+ if (me.tags) {
+ me.loadTags(me.tags);
+ }
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 08/14] ui: add form/TagEdit.js
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 08/14] ui: add form/TagEdit.js Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
0 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Out of curiosity, did you decide to use the older style of component, instead of
the newer declarative approach, to be able to have references to the buttons as
"me.<button>" instead of having to do lookups all the time?
one more thing inline
On 6/21/22 11:20, Dominik Csapak wrote:
> this is a wrapper container for holding a list of (editable) tags
> intended to be used in the lxc/qemu status toolbar
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/form/TagEdit.js | 151 +++++++++++++++++++++++++++++++++++
> 2 files changed, 152 insertions(+)
> create mode 100644 www/manager6/form/TagEdit.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 9d610f71..eb4be4c5 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -75,6 +75,7 @@ JSSRC= \
> form/iScsiProviderSelector.js \
> form/TagColorGrid.js \
> form/Tag.js \
> + form/TagEdit.js \
> grid/BackupView.js \
> grid/FirewallAliases.js \
> grid/FirewallOptions.js \
> diff --git a/www/manager6/form/TagEdit.js b/www/manager6/form/TagEdit.js
> new file mode 100644
> index 00000000..5a267169
> --- /dev/null
> +++ b/www/manager6/form/TagEdit.js
> @@ -0,0 +1,151 @@
> +Ext.define('PVE.panel.TagEditContainer', {
> + extend: 'Ext.container.Container',
> + alias: 'widget.pveTagEditContainer',
> +
> + tagCount: 0,
> +
> + layout: {
> + type: 'hbox',
> + align: 'stretch',
> + },
> +
> + loadTags: function(tagstring = '', inEdit, force = false) {
> + let me = this;
> +
> + if (me.oldTags === tagstring && !force) {
> + return;
> + }
> +
> + let tags = tagstring.split(/[;, ]/).filter((t) => !!t) || [];
This is used at least 3 times throughout this series. Would it make sense to put
it into a small util helper function to have it in one place?
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH manager v7 09/14] ui: {lxc, qemu}/Config: show Tags and make them editable
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (17 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 08/14] ui: add form/TagEdit.js Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree Dominik Csapak
` (6 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
add the tags in the status line, and add a button for adding new ones
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/lxc/Config.js | 32 ++++++++++++++++++++++++++++++--
www/manager6/qemu/Config.js | 31 +++++++++++++++++++++++++++++--
2 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 89b59c9b..9f1994d3 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -4,6 +4,8 @@ Ext.define('PVE.lxc.Config', {
onlineHelp: 'chapter_pct',
+ userCls: 'proxmox-tags-full',
+
initComponent: function() {
var me = this;
var vm = me.pveSelNode.data;
@@ -182,12 +184,33 @@ Ext.define('PVE.lxc.Config', {
],
});
+ let tagsContainer = Ext.create('PVE.panel.TagEditContainer', {
+ tags: vm.tags,
+ listeners: {
+ change: function(tags) {
+ Proxmox.Utils.API2Request({
+ url: base_url + '/config',
+ method: 'PUT',
+ params: {
+ tags,
+ },
+ success: function() {
+ me.statusStore.load();
+ },
+ failure: function(response) {
+ Ext.Msg.alert('Error', response.htmlStatus);
+ me.statusStore.load();
+ },
+ });
+ },
+ },
+ });
Ext.apply(me, {
title: Ext.String.format(gettext("Container {0} on node '{1}'"), vm.text, nodename),
hstateid: 'lxctab',
tbarSpacing: false,
- tbar: [statusTxt, '->', startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
+ tbar: [statusTxt, tagsContainer, '->', startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
defaults: { statusStore: me.statusStore },
items: [
{
@@ -344,10 +367,12 @@ Ext.define('PVE.lxc.Config', {
me.mon(me.statusStore, 'load', function(s, records, success) {
var status;
var lock;
+ var rec;
+
if (!success) {
status = 'unknown';
} else {
- var rec = s.data.get('status');
+ rec = s.data.get('status');
status = rec ? rec.data.value : 'unknown';
rec = s.data.get('template');
template = rec ? rec.data.value : false;
@@ -357,6 +382,9 @@ Ext.define('PVE.lxc.Config', {
statusTxt.update({ lock: lock });
+ rec = s.data.get('tags');
+ tagsContainer.loadTags(rec?.data?.value);
+
startBtn.setDisabled(!caps.vms['VM.PowerMgmt'] || status === 'running' || template);
shutdownBtn.setDisabled(!caps.vms['VM.PowerMgmt'] || status !== 'running');
me.down('#removeBtn').setDisabled(!caps.vms['VM.Allocate'] || status !== 'stopped');
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 9fe933df..2cd6d856 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -3,6 +3,7 @@ Ext.define('PVE.qemu.Config', {
alias: 'widget.PVE.qemu.Config',
onlineHelp: 'chapter_virtual_machines',
+ userCls: 'proxmox-tags-full',
initComponent: function() {
var me = this;
@@ -219,11 +220,33 @@ Ext.define('PVE.qemu.Config', {
],
});
+ let tagsContainer = Ext.create('PVE.panel.TagEditContainer', {
+ tags: vm.tags,
+ listeners: {
+ change: function(tags) {
+ Proxmox.Utils.API2Request({
+ url: base_url + '/config',
+ method: 'PUT',
+ params: {
+ tags,
+ },
+ success: function() {
+ me.statusStore.load();
+ },
+ failure: function(response) {
+ Ext.Msg.alert('Error', response.htmlStatus);
+ me.statusStore.load();
+ },
+ });
+ },
+ },
+ });
+
Ext.apply(me, {
title: Ext.String.format(gettext("Virtual Machine {0} on node '{1}'"), vm.text, nodename),
hstateid: 'kvmtab',
tbarSpacing: false,
- tbar: [statusTxt, '->', resumeBtn, startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
+ tbar: [statusTxt, tagsContainer, '->', resumeBtn, startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
defaults: { statusStore: me.statusStore },
items: [
{
@@ -382,11 +405,12 @@ Ext.define('PVE.qemu.Config', {
var spice = false;
var xtermjs = false;
var lock;
+ var rec;
if (!success) {
status = qmpstatus = 'unknown';
} else {
- var rec = s.data.get('status');
+ rec = s.data.get('status');
status = rec ? rec.data.value : 'unknown';
rec = s.data.get('qmpstatus');
qmpstatus = rec ? rec.data.value : 'unknown';
@@ -399,6 +423,9 @@ Ext.define('PVE.qemu.Config', {
xtermjs = !!s.data.get('serial');
}
+ rec = s.data.get('tags');
+ tagsContainer.loadTags(rec?.data?.value);
+
if (template) {
return;
}
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (18 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 09/14] ui: {lxc, qemu}/Config: show Tags and make them editable Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 11/14] ui: form/GlobalSearchField: display tags and allow to search for them Dominik Csapak
` (5 subsequent siblings)
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/lxc/Config.js | 4 +++-
www/manager6/qemu/Config.js | 4 +++-
www/manager6/tree/ResourceTree.js | 4 ++++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 9f1994d3..1b79628e 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -206,8 +206,10 @@ Ext.define('PVE.lxc.Config', {
},
});
+ let vm_text = `${vm.vmid} (${vm.name})`;
+
Ext.apply(me, {
- title: Ext.String.format(gettext("Container {0} on node '{1}'"), vm.text, nodename),
+ title: Ext.String.format(gettext("Container {0} on node '{1}'"), vm_text, nodename),
hstateid: 'lxctab',
tbarSpacing: false,
tbar: [statusTxt, tagsContainer, '->', startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 2cd6d856..5c8fa620 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -242,8 +242,10 @@ Ext.define('PVE.qemu.Config', {
},
});
+ let vm_text = `${vm.vmid} (${vm.name})`;
+
Ext.apply(me, {
- title: Ext.String.format(gettext("Virtual Machine {0} on node '{1}'"), vm.text, nodename),
+ title: Ext.String.format(gettext("Virtual Machine {0} on node '{1}'"), vm_text, nodename),
hstateid: 'kvmtab',
tbarSpacing: false,
tbar: [statusTxt, tagsContainer, '->', resumeBtn, startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index 139defab..d41721b9 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -5,6 +5,8 @@ Ext.define('PVE.tree.ResourceTree', {
extend: 'Ext.tree.TreePanel',
alias: ['widget.pveResourceTree'],
+ userCls: 'proxmox-tags-circle',
+
statics: {
typeDefaults: {
node: {
@@ -114,6 +116,8 @@ Ext.define('PVE.tree.ResourceTree', {
}
}
+ info.text += PVE.Utils.renderTags(info.tags, PVE.Utils.tagOverrides);
+
info.text = status + info.text;
},
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
2022-09-15 11:54 ` Dominik Csapak
0 siblings, 1 reply; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Why the change from vm.text to vm_text in {lxc,qemu}/Config.js?
AFAICT we have exactly the same string in the now not used "vm.text".
If these changes are needed and should be part of this commmit, a short
explanation would be good as it does not seem to have anything to do with the
resource tree.
On 6/21/22 11:20, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/lxc/Config.js | 4 +++-
> www/manager6/qemu/Config.js | 4 +++-
> www/manager6/tree/ResourceTree.js | 4 ++++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
> index 9f1994d3..1b79628e 100644
> --- a/www/manager6/lxc/Config.js
> +++ b/www/manager6/lxc/Config.js
> @@ -206,8 +206,10 @@ Ext.define('PVE.lxc.Config', {
> },
> });
>
> + let vm_text = `${vm.vmid} (${vm.name})`;
> +
> Ext.apply(me, {
> - title: Ext.String.format(gettext("Container {0} on node '{1}'"), vm.text, nodename),
> + title: Ext.String.format(gettext("Container {0} on node '{1}'"), vm_text, nodename),
> hstateid: 'lxctab',
> tbarSpacing: false,
> tbar: [statusTxt, tagsContainer, '->', startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
> diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
> index 2cd6d856..5c8fa620 100644
> --- a/www/manager6/qemu/Config.js
> +++ b/www/manager6/qemu/Config.js
> @@ -242,8 +242,10 @@ Ext.define('PVE.qemu.Config', {
> },
> });
>
> + let vm_text = `${vm.vmid} (${vm.name})`;
> +
> Ext.apply(me, {
> - title: Ext.String.format(gettext("Virtual Machine {0} on node '{1}'"), vm.text, nodename),
> + title: Ext.String.format(gettext("Virtual Machine {0} on node '{1}'"), vm_text, nodename),
> hstateid: 'kvmtab',
> tbarSpacing: false,
> tbar: [statusTxt, tagsContainer, '->', resumeBtn, startBtn, shutdownBtn, migrateBtn, consoleBtn, moreBtn],
> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
> index 139defab..d41721b9 100644
> --- a/www/manager6/tree/ResourceTree.js
> +++ b/www/manager6/tree/ResourceTree.js
> @@ -5,6 +5,8 @@ Ext.define('PVE.tree.ResourceTree', {
> extend: 'Ext.tree.TreePanel',
> alias: ['widget.pveResourceTree'],
>
> + userCls: 'proxmox-tags-circle',
> +
> statics: {
> typeDefaults: {
> node: {
> @@ -114,6 +116,8 @@ Ext.define('PVE.tree.ResourceTree', {
> }
> }
>
> + info.text += PVE.Utils.renderTags(info.tags, PVE.Utils.tagOverrides);
> +
> info.text = status + info.text;
> },
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree
2022-09-14 14:15 ` Aaron Lauterer
@ 2022-09-15 11:54 ` Dominik Csapak
0 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-09-15 11:54 UTC (permalink / raw)
To: Aaron Lauterer, Proxmox VE development discussion
On 9/14/22 16:15, Aaron Lauterer wrote:
> Why the change from vm.text to vm_text in {lxc,qemu}/Config.js?
>
> AFAICT we have exactly the same string in the now not used "vm.text".
>
> If these changes are needed and should be part of this commmit, a short explanation would be good as
> it does not seem to have anything to do with the resource tree.
you're right in that the explanation is missing:
to show the tags in the tree, we have to modify the text in those records, which are passed
through to the components and end up in 'vm.text', so the tags would end up there
but since we want the tags to be shown differently there (to be able to add/edit them)
we cannot use that anymore
>
> On 6/21/22 11:20, Dominik Csapak wrote:
[snip]
>> node: {
>> @@ -114,6 +116,8 @@ Ext.define('PVE.tree.ResourceTree', {
>> }
>> }
>> + info.text += PVE.Utils.renderTags(info.tags, PVE.Utils.tagOverrides);
>> +
>> info.text = status + info.text;
>> },
here we modify the 'text' property
^ permalink raw reply [flat|nested] 41+ messages in thread
* [pve-devel] [PATCH manager v7 11/14] ui: form/GlobalSearchField: display tags and allow to search for them
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (19 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 12/14] ui: form/Tag: add 'admin-tag' class to admin tags Dominik Csapak
` (4 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
each tag is treated like a seperate field, so it weighs more if the user
searches for the exact string of a single tag
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/form/GlobalSearchField.js | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/www/manager6/form/GlobalSearchField.js b/www/manager6/form/GlobalSearchField.js
index 267a480d..8e815d4f 100644
--- a/www/manager6/form/GlobalSearchField.js
+++ b/www/manager6/form/GlobalSearchField.js
@@ -15,6 +15,7 @@ Ext.define('PVE.form.GlobalSearchField', {
grid: {
xtype: 'gridpanel',
+ userCls: 'proxmox-tags-full',
focusOnToFront: false,
floating: true,
emptyText: Proxmox.Utils.noneText,
@@ -23,7 +24,7 @@ Ext.define('PVE.form.GlobalSearchField', {
scrollable: {
xtype: 'scroller',
y: true,
- x: false,
+ x: true,
},
store: {
model: 'PVEResources',
@@ -78,6 +79,11 @@ Ext.define('PVE.form.GlobalSearchField', {
text: gettext('Description'),
flex: 1,
dataIndex: 'text',
+ renderer: function(value, mD, rec) {
+ let overrides = PVE.Utils.tagOverrides;
+ let tags = PVE.Utils.renderTags(rec.data.tags, overrides);
+ return `${value}${tags}`;
+ },
},
{
text: gettext('Node'),
@@ -104,16 +110,20 @@ Ext.define('PVE.form.GlobalSearchField', {
'storage': ['type', 'pool', 'node', 'storage'],
'default': ['name', 'type', 'node', 'pool', 'vmid'],
};
- let fieldArr = fieldMap[item.data.type] || fieldMap.default;
+ let fields = fieldMap[item.data.type] || fieldMap.default;
+ let fieldArr = fields.map(field => item.data[field]?.toString().toLowerCase());
+ if (item.data.tags) {
+ let tags = item.data.tags.split(/[;, ]/);
+ fieldArr.push(...tags);
+ }
let filterWords = me.filterVal.split(/\s+/);
// all text is case insensitive and each split-out word is searched for separately.
// a row gets 1 point for every partial match, and and additional point for every exact match
let match = 0;
- for (let field of fieldArr) {
- let fieldValue = item.data[field]?.toString().toLowerCase();
- if (fieldValue === undefined) {
+ for (let fieldValue of fieldArr) {
+ if (fieldValue === undefined || fieldValue === "") {
continue;
}
for (let filterWord of filterWords) {
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 12/14] ui: form/Tag: add 'admin-tag' class to admin tags
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (20 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 11/14] ui: form/GlobalSearchField: display tags and allow to search for them Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 13/14] ui: ResourceGrid: render tags Dominik Csapak
` (3 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
so that they are emphasized there too
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/form/Tag.js | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index 0a754edb..91190051 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -141,6 +141,7 @@ Ext.define('Proxmox.Tag', {
setText: function(text) {
let me = this;
me.tagEl().innerHTML = text;
+ me.tagEl().classList = text[0] === '+' ? 'admin-tag' : '';
me.layoutCallback();
},
@@ -256,8 +257,9 @@ Ext.define('Proxmox.Tag', {
let iconStyle = me.mode !== 'editable' ? 'display: none' : 'padding-right: 6px;';
let iconCls = me.icons[me.addTag ? 'addTag' : me.mode];
+ let adminCls = me.tag[0] === '+' ? 'admin-tag' : '';
let icon = ` <i style="cursor: pointer; ${iconStyle}" class="fa fa-${iconCls}${me.faIconStyle}"></i>`;
- me.html = `<span style="padding-right: 1px">${me.tag}</span>${icon}`;
+ me.html = `<span style="padding-right: 1px" class="${adminCls}">${me.tag}</span>${icon}`;
me.callParent();
if (me.addTag) {
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 13/14] ui: ResourceGrid: render tags
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (21 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 12/14] ui: form/Tag: add 'admin-tag' class to admin tags Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags Dominik Csapak
` (2 subsequent siblings)
25 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
with the 'full' styling
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/data/ResourceStore.js | 3 +++
www/manager6/grid/ResourceGrid.js | 1 +
2 files changed, 4 insertions(+)
diff --git a/www/manager6/data/ResourceStore.js b/www/manager6/data/ResourceStore.js
index b18f7dd8..0bfe9259 100644
--- a/www/manager6/data/ResourceStore.js
+++ b/www/manager6/data/ResourceStore.js
@@ -295,6 +295,9 @@ Ext.define('PVE.data.ResourceStore', {
},
tags: {
header: gettext('Tags'),
+ renderer: (value) => {
+ return PVE.Utils.renderTags(value, PVE.Utils.tagOverrides);
+ },
type: 'string',
hidden: true,
sortable: true,
diff --git a/www/manager6/grid/ResourceGrid.js b/www/manager6/grid/ResourceGrid.js
index 29906a37..9376bcc2 100644
--- a/www/manager6/grid/ResourceGrid.js
+++ b/www/manager6/grid/ResourceGrid.js
@@ -7,6 +7,7 @@ Ext.define('PVE.grid.ResourceGrid', {
property: 'type',
direction: 'ASC',
},
+ userCls: 'proxmox-tags-full',
initComponent: function() {
let me = this;
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (22 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 13/14] ui: ResourceGrid: render tags Dominik Csapak
@ 2022-06-21 9:20 ` Dominik Csapak
2022-09-14 14:15 ` Aaron Lauterer
2022-09-14 14:34 ` [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Aaron Lauterer
2022-09-16 7:19 ` Thomas Lamprecht
25 siblings, 1 reply; 41+ messages in thread
From: Dominik Csapak @ 2022-06-21 9:20 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/form/Tag.js | 22 +++++++--
www/manager6/form/TagEdit.js | 96 +++++++++++++++++++++++++++++++++++-
2 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index 91190051..dcbd9597 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -31,6 +31,9 @@ Ext.define('Proxmox.Tag', {
if (event.target.tagName !== 'I') {
return;
}
+ if (event.target.classList.contains('handle')) {
+ return;
+ }
switch (me.mode) {
case 'editable':
if (me.addTag) {
@@ -156,12 +159,14 @@ Ext.define('Proxmox.Tag', {
let text = me.tag;
let cursor = 'pointer';
let padding = '0px';
+ let dragHandleStyle = 'none';
switch (mode) {
case 'normal':
iconStyle += 'display: none;';
padding = undefined;
break;
case 'editable':
+ dragHandleStyle = '';
break;
case 'edit':
me.tagEl().contentEditable = true;
@@ -174,12 +179,14 @@ Ext.define('Proxmox.Tag', {
if (me.addTag) {
me.setText(text);
me.setStyle('cursor', cursor);
+ dragHandleStyle = 'none';
}
me.setStyle('padding-right', padding);
me.iconEl().classList = `fa fa-${icon}${me.faIconStyle}`;
me.iconEl().style = iconStyle;
+ me.dragEl().style.display = dragHandleStyle;
me.mode = mode;
},
@@ -233,14 +240,18 @@ Ext.define('Proxmox.Tag', {
}
},
- tagEl: function() {
+ dragEl: function() {
return this.el?.dom?.children?.[0];
},
- iconEl: function() {
+ tagEl: function() {
return this.el?.dom?.children?.[1];
},
+ iconEl: function() {
+ return this.el?.dom?.children?.[2];
+ },
+
initComponent: function() {
let me = this;
if (me.tag === undefined && !me.addTag) {
@@ -256,10 +267,15 @@ Ext.define('Proxmox.Tag', {
let iconStyle = me.mode !== 'editable' ? 'display: none' : 'padding-right: 6px;';
let iconCls = me.icons[me.addTag ? 'addTag' : me.mode];
+ let dragHandleStyle = 'cursor: grab; font-size: 14px;';
+ if (me.addTag || me.mode !== 'editable') {
+ dragHandleStyle += 'display: none';
+ }
let adminCls = me.tag[0] === '+' ? 'admin-tag' : '';
+ let dragHandle = `<i class="handle fa fa-bars" style="${dragHandleStyle}"></i> `;
let icon = ` <i style="cursor: pointer; ${iconStyle}" class="fa fa-${iconCls}${me.faIconStyle}"></i>`;
- me.html = `<span style="padding-right: 1px" class="${adminCls}">${me.tag}</span>${icon}`;
+ me.html = `${dragHandle}<span style="padding-right: 1px" class="${adminCls}">${me.tag}</span>${icon}`;
me.callParent();
if (me.addTag) {
diff --git a/www/manager6/form/TagEdit.js b/www/manager6/form/TagEdit.js
index 5a267169..85f9f63e 100644
--- a/www/manager6/form/TagEdit.js
+++ b/www/manager6/form/TagEdit.js
@@ -18,7 +18,7 @@ Ext.define('PVE.panel.TagEditContainer', {
let tags = tagstring.split(/[;, ]/).filter((t) => !!t) || [];
me.suspendLayout = true;
- me.tags = {};
+ me.tagList = [];
me.removeAllTags();
tags.forEach((tag) => {
me.addTag(tag, inEdit);
@@ -87,6 +87,7 @@ Ext.define('PVE.panel.TagEditContainer', {
me.remove(tag);
});
me.tagFields = [];
+ me.tagList = [];
me.noTagsField.setVisible(true);
},
@@ -94,11 +95,13 @@ Ext.define('PVE.panel.TagEditContainer', {
let me = this;
let tagField = me.insert(me.tagFields.length + 1, {
xtype: 'pmxTag',
+ tagIndex: me.tagList.length,
tag,
mode: inEdit ? 'editable' : 'normal',
layoutCallback: () => me.updateLayout(),
});
me.tagFields.push(tagField);
+ me.tagList.push(tag);
me.noTagsField.setVisible(false);
},
@@ -147,5 +150,96 @@ Ext.define('PVE.panel.TagEditContainer', {
if (me.tags) {
me.loadTags(me.tags);
}
+
+ me.on('render', function(v) {
+ me.dragzone = Ext.create('Ext.dd.DragZone', v.getEl(), {
+ getDragData: function(e) {
+ let source = e.getTarget('.handle');
+ if (source) {
+ let sourceId = source.parentNode.id;
+ let cmp = Ext.getCmp(sourceId);
+ let tag = cmp.getTag();
+ let ddel = document.createElement('div');
+ ddel.classList.add('proxmox-tags-full');
+ ddel.innerHTML = Proxmox.Utils.getTagElement(tag, PVE.Utils.tagOverrides);
+ let repairXY = Ext.fly(source).getXY();
+ cmp.setDisabled(true);
+ ddel.id = Ext.id();
+ return {
+ ddel,
+ repairXY,
+ tagIndex: cmp.tagIndex,
+ tag: cmp.getTag(),
+ sourceId,
+ };
+ }
+ return undefined;
+ },
+ onMouseUp: function(target, e, id) {
+ let cmp = Ext.getCmp(this.dragData.sourceId);
+ if (cmp && !cmp.isDestroyed) {
+ cmp.setDisabled(false);
+ }
+ },
+ getRepairXY: function() {
+ return this.dragData.repairXY;
+ },
+ beforeInvalidDrop: function(target, e, id) {
+ let cmp = Ext.getCmp(this.dragData.sourceId);
+ if (cmp && !cmp.isDestroyed) {
+ cmp.setDisabled(false);
+ }
+ },
+ });
+ me.dropzone = Ext.create('Ext.dd.DropZone', v.getEl(), {
+ getTargetFromEvent: function(e) {
+ return e.getTarget('.proxmox-tag-dark,.proxmox-tag-light');
+ },
+ getIndicator: function() {
+ if (!me.indicator) {
+ me.indicator = Ext.create('Ext.Component', {
+ floating: true,
+ html: '<i class="fa fa-long-arrow-up"></i>',
+ hidden: true,
+ shadow: false,
+ });
+ }
+ return me.indicator;
+ },
+ onContainerOver: function() {
+ this.getIndicator().setVisible(false);
+ },
+ notifyOut: function() {
+ this.getIndicator().setVisible(false);
+ },
+ onNodeOver: function(target, dd, e, data) {
+ let indicator = this.getIndicator();
+ indicator.setVisible(true);
+ indicator.alignTo(Ext.getCmp(target.id), 't50-bl', [-1, -2]);
+ return this.dropAllowed;
+ },
+ onNodeDrop: function(target, dd, e, data) {
+ this.getIndicator().setVisible(false);
+ let tagIdx = data.tagIndex;
+ let tag = data.tag;
+ me.tagList.splice(tagIdx, 1);
+ let targetCmp = Ext.getCmp(target.id);
+ let targetIdx = targetCmp.tagIndex ?? Infinity;
+ if (targetIdx > tagIdx) {
+ targetIdx--;
+ }
+ me.tagList.splice(targetIdx, 0, tag);
+ me.loadTags(me.tagList.join(','), true, true);
+ },
+ });
+ });
+ },
+
+ destroy: function() {
+ let me = this;
+ Ext.destroy(me.dragzone);
+ Ext.destroy(me.dropzone);
+ Ext.destroy(me.indicator);
+ me.callParent();
},
});
--
2.30.2
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags Dominik Csapak
@ 2022-09-14 14:15 ` Aaron Lauterer
2022-09-15 11:56 ` Dominik Csapak
0 siblings, 1 reply; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:15 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
On 6/21/22 11:20, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/form/Tag.js | 22 +++++++--
> www/manager6/form/TagEdit.js | 96 +++++++++++++++++++++++++++++++++++-
> 2 files changed, 114 insertions(+), 4 deletions(-)
>
> diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
> index 91190051..dcbd9597 100644
> --- a/www/manager6/form/Tag.js
> +++ b/www/manager6/form/Tag.js
> @@ -31,6 +31,9 @@ Ext.define('Proxmox.Tag', {
> if (event.target.tagName !== 'I') {
> return;
> }
> + if (event.target.classList.contains('handle')) {
> + return;
> + }
> switch (me.mode) {
> case 'editable':
> if (me.addTag) {
> @@ -156,12 +159,14 @@ Ext.define('Proxmox.Tag', {
> let text = me.tag;
> let cursor = 'pointer';
> let padding = '0px';
> + let dragHandleStyle = 'none';
> switch (mode) {
> case 'normal':
> iconStyle += 'display: none;';
> padding = undefined;
> break;
> case 'editable':
> + dragHandleStyle = '';
Is there a reason for the '' here compared to the 'none' above and below?
> break;
> case 'edit':
> me.tagEl().contentEditable = true;
> @@ -174,12 +179,14 @@ Ext.define('Proxmox.Tag', {
> if (me.addTag) {
> me.setText(text);
> me.setStyle('cursor', cursor);
> + dragHandleStyle = 'none';
> }
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags
2022-09-14 14:15 ` Aaron Lauterer
@ 2022-09-15 11:56 ` Dominik Csapak
0 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-09-15 11:56 UTC (permalink / raw)
To: Aaron Lauterer, Proxmox VE development discussion
On 9/14/22 16:15, Aaron Lauterer wrote:
>
>
> On 6/21/22 11:20, Dominik Csapak wrote:
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> www/manager6/form/Tag.js | 22 +++++++--
>> www/manager6/form/TagEdit.js | 96 +++++++++++++++++++++++++++++++++++-
>> 2 files changed, 114 insertions(+), 4 deletions(-)
>>
>> diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
>> index 91190051..dcbd9597 100644
>> --- a/www/manager6/form/Tag.js
>> +++ b/www/manager6/form/Tag.js
>> @@ -31,6 +31,9 @@ Ext.define('Proxmox.Tag', {
>> if (event.target.tagName !== 'I') {
>> return;
>> }
>> + if (event.target.classList.contains('handle')) {
>> + return;
>> + }
>> switch (me.mode) {
>> case 'editable':
>> if (me.addTag) {
>> @@ -156,12 +159,14 @@ Ext.define('Proxmox.Tag', {
>> let text = me.tag;
>> let cursor = 'pointer';
>> let padding = '0px';
>> + let dragHandleStyle = 'none';
>> switch (mode) {
>> case 'normal':
>> iconStyle += 'display: none;';
>> padding = undefined;
>> break;
>> case 'editable':
>> + dragHandleStyle = '';
>
> Is there a reason for the '' here compared to the 'none' above and below?
yes, to show it, we set
style.display = dragHandleStyle
which if '', shows it ('none' hides it)
>
>> break;
>> case 'edit':
>> me.tagEl().contentEditable = true;
>> @@ -174,12 +179,14 @@ Ext.define('Proxmox.Tag', {
>> if (me.addTag) {
>> me.setText(text);
>> me.setStyle('cursor', cursor);
>> + dragHandleStyle = 'none';
>> }
>
> [...]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (23 preceding siblings ...)
2022-06-21 9:20 ` [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags Dominik Csapak
@ 2022-09-14 14:34 ` Aaron Lauterer
2022-09-16 7:19 ` Thomas Lamprecht
25 siblings, 0 replies; 41+ messages in thread
From: Aaron Lauterer @ 2022-09-14 14:34 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Tested it and a few things I noticed:
- can add one tag multiple times
- user with PVEAdmin role on / (should not have Sys.Modify permissions) can
remove an admin tag, though it does not work if I apply VMAdmin role privileges
directly on the VM
- editing tags themselves seems to be a bit buggy, more details in the reply to
pve-manager 5/14 add form/Tag.
-> though it seems that it might be a firefox addon in my case, as it works
much better in a private window. need to investigate that more.
other than that it works well.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui
2022-06-21 9:19 [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Dominik Csapak
` (24 preceding siblings ...)
2022-09-14 14:34 ` [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui Aaron Lauterer
@ 2022-09-16 7:19 ` Thomas Lamprecht
2022-09-16 7:50 ` Dominik Csapak
25 siblings, 1 reply; 41+ messages in thread
From: Thomas Lamprecht @ 2022-09-16 7:19 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 21/06/2022 um 11:19 schrieb Dominik Csapak:
> this series brings the already existing 'tags' for ct/vms to the gui:
> * tags can be edited in the status toolbar of the guest
> * existing tags will be shown in the tree/global search/resource grids
> * when editing a tag, a list of existing tags will be shown
> * by default, the color is (consistently) autogenerated based on the
> text
> * that color can be overriden in datacenter -> options (cluster wide)
> (edit window for browser local storage is TBD)
> * by default, the text color is either white or black, depending which
> provides the greater contrast (according to SAPC)
> * this text color can also be overridden
> * there are multiple shapes available for the tree (see [0])
> * adds new 'admin' tags that need higher privliges, these can then be
> used to enable features like 'inlude in backup by tag', etc.
I didn't find the permission semantics for non-admin tags when skimming
the series, but after thinking about it off an on again recently I figure
that it could be seen as quite problematic in general to even assign existing
tags, then visible in the resource view, if the user got only limited privileges
to a VM; and that it could be quite problematic for such users to create new
ones, allowing typo squatting, slurs, ...? to have ill effects to other users
or admins of the same system/cluster. I mean to a certain degree this can be
correlated to the permission semantics of the hostname, but still there's more
of them and they're way flashier.
One method could be (always talking about non-admin tags for now):
* allow unrestricted usage of those for users with Sys.Modify on / + the rights
on the object to modify (i.e., for guests that would be VM.Modify on /vms/<vmid>)
* for users without the powerful Sys.Modify on / we could give the admins the decision,
for example through a datactenter.cfg property that could work like:
user-tag-privs: useable=<none|existing|free|list>[,list=<tag1;tag2;...>]
Meaning:
- useable=none -> only users with powerful Sys.Modify -> / can configure tags
- useable=existing -> currently existing tags can be used, the list could be added
as base-set (so that users that only got one VM can actually set some)
- useable=free -> existing can be used freely and new ones can be added freely,
the list would only act as base suggestion set.
- useable=list -> well, the list reflects all allowed tags to use (independent
if they exist for that users POV or don't).
This would be still orthogonal to admin:tags (as their purpose is to avoid users
adding a possible OK tag to a unwanted guest due to actions like backup using said
tag).
ps, semi-related. I'd like (as it not strictly insist on that, but I find it a bit
too ugly to do) to not further extend the already misused /version call if possible.
I just find it unidiomatic and adding a new one now with the current extra info would
be possible, we then could remove that extra info from /version with the next major
release.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [pve-devel] [PATCH common/cluster/qemu/container/wt/manager v7] add tags to ui
2022-09-16 7:19 ` Thomas Lamprecht
@ 2022-09-16 7:50 ` Dominik Csapak
0 siblings, 0 replies; 41+ messages in thread
From: Dominik Csapak @ 2022-09-16 7:50 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 9/16/22 09:19, Thomas Lamprecht wrote:
> Am 21/06/2022 um 11:19 schrieb Dominik Csapak:
>> this series brings the already existing 'tags' for ct/vms to the gui:
>> * tags can be edited in the status toolbar of the guest
>> * existing tags will be shown in the tree/global search/resource grids
>> * when editing a tag, a list of existing tags will be shown
>> * by default, the color is (consistently) autogenerated based on the
>> text
>> * that color can be overriden in datacenter -> options (cluster wide)
>> (edit window for browser local storage is TBD)
>> * by default, the text color is either white or black, depending which
>> provides the greater contrast (according to SAPC)
>> * this text color can also be overridden
>> * there are multiple shapes available for the tree (see [0])
>> * adds new 'admin' tags that need higher privliges, these can then be
>> used to enable features like 'inlude in backup by tag', etc.
>
> I didn't find the permission semantics for non-admin tags when skimming
> the series, but after thinking about it off an on again recently I figure
> that it could be seen as quite problematic in general to even assign existing
> tags, then visible in the resource view, if the user got only limited privileges
> to a VM; and that it could be quite problematic for such users to create new
> ones, allowing typo squatting, slurs, ...? to have ill effects to other users
> or admins of the same system/cluster. I mean to a certain degree this can be
> correlated to the permission semantics of the hostname, but still there's more
> of them and they're way flashier.
thats true of course, the current tags require 'VM.Config.Options' privileges,
so anybody who has some edit access to vms can add tags there
>
> One method could be (always talking about non-admin tags for now):
>
> * allow unrestricted usage of those for users with Sys.Modify on / + the rights
> on the object to modify (i.e., for guests that would be VM.Modify on /vms/<vmid>)
>
> * for users without the powerful Sys.Modify on / we could give the admins the decision,
> for example through a datactenter.cfg property that could work like:
>
> user-tag-privs: useable=<none|existing|free|list>[,list=<tag1;tag2;...>]
>
> Meaning:
> - useable=none -> only users with powerful Sys.Modify -> / can configure tags
> - useable=existing -> currently existing tags can be used, the list could be added
> as base-set (so that users that only got one VM can actually set some)
> - useable=free -> existing can be used freely and new ones can be added freely,
> the list would only act as base suggestion set.
> - useable=list -> well, the list reflects all allowed tags to use (independent
> if they exist for that users POV or don't).
this looks nice, the default would have to be usable=free for it to be not a breaking
change though (or is it?), could be changed ofc with the next major release
>
> This would be still orthogonal to admin:tags (as their purpose is to avoid users
> adding a possible OK tag to a unwanted guest due to actions like backup using said
> tag).
we could also add a list of predefined admin tags here too (either as seperate option,
or as second list in the property string), that would only be settable by users with
'Sys.Modify' on /
that is what i suggested to Aarons review regarding admin confusion
would that make more sense than using some prefix maybe?
>
> ps, semi-related. I'd like (as it not strictly insist on that, but I find it a bit
> too ugly to do) to not further extend the already misused /version call if possible.
> I just find it unidiomatic and adding a new one now with the current extra info would
> be possible, we then could remove that extra info from /version with the next major
> release.
yes, a 'datacenter-info' (or similar, it was the first thing that popped into my mind)
would be nicer
^ permalink raw reply [flat|nested] 41+ messages in thread