[pve-devel] [PATCH installer 0/1] fix #4869: Show state in management interface ComboBox

[pve-devel] [PATCH installer 0/1] fix #4869: Show state in management interface ComboBox

[Filip Schauer]

This can be tested by modifying an existing Proxmox-VE ISO file as
described in the following forum post:

https://forum.proxmox.com/threads/create-custom-pve-iso-from-original-pve-iso.123606/post-538612

Filip Schauer (1):
  fix #4869: Show state in management interface ComboBox

 proxinstall | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.39.2

[pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxinstall | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxinstall b/proxinstall
index d5b2565..9316578 100755
--- a/proxinstall
+++ b/proxinstall
@@ -347,7 +347,7 @@ sub create_ipconf_view {
 
     my $get_device_desc = sub {
 	my $iface = shift;
-	return "$iface->{name} - $iface->{mac} ($iface->{driver})";
+	return "$iface->{name} - $iface->{mac} ($iface->{driver}) - $iface->{state}";
     };
 
     my $run_env = Proxmox::Install::RunEnv::get();
-- 
2.39.2

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Christoph Heiss]

Needs to be done for the TUI installer as well. Easiest way here is to
add `state` to each interface in Proxmox::Sys::RunEnv::query_netdevs(),
then deserialize that in `setup::Interface`.

Looking at it, adding the MAC address there too would be great, if you
are already at it.

On Fri, Aug 04, 2023 at 12:26:46PM +0200, Filip Schauer wrote:
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  proxinstall | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/proxinstall b/proxinstall
> index d5b2565..9316578 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -347,7 +347,7 @@ sub create_ipconf_view {
>
>      my $get_device_desc = sub {
>  	my $iface = shift;
> -	return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> +	return "$iface->{name} - $iface->{mac} ($iface->{driver}) - $iface->{state}";
I think it would be better to only show the state if the interface is
actually UP - thus drawing immediate attention to these list entries and
not completely overloading that dropdown.

Maybe even append it as "(UP)" instead of by dash (as it does not really
identify the interface, but rather is a property of it).

>      };
>
>      my $run_env = Proxmox::Install::RunEnv::get();
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Thomas Lamprecht]

Am 10/10/2023 um 13:33 schrieb Christoph Heiss:
> On Fri, Aug 04, 2023 at 12:26:46PM +0200, Filip Schauer wrote:
>> @@ -347,7 +347,7 @@ sub create_ipconf_view {
>>
>>      my $get_device_desc = sub {
>>  	my $iface = shift;
>> -	return "$iface->{name} - $iface->{mac} ($iface->{driver})";
>> +	return "$iface->{name} - $iface->{mac} ($iface->{driver}) - $iface->{state}";
>
> I think it would be better to only show the state if the interface is
> actually UP - thus drawing immediate attention to these list entries and
> not completely overloading that dropdown.
> 
> Maybe even append it as "(UP)" instead of by dash (as it does not really
> identify the interface, but rather is a property of it).
> 

Or, if the GTK component and available fonts support it we could also use
unicode

UP   -> 🟢 https://unicode-explorer.com/c/1F7E2
DOWN -> ◯ (preferred, easier to differ for vision impaired people) or ⬤
        https://unicode-explorer.com/c/25EF or https://unicode-explorer.com/c/2B24

Would use less space and probably look better, if it works that is ^^

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Christoph Heiss]

On Tue, Oct 10, 2023 at 01:55:43PM +0200, Thomas Lamprecht wrote:
>
> Am 10/10/2023 um 13:33 schrieb Christoph Heiss:
> > On Fri, Aug 04, 2023 at 12:26:46PM +0200, Filip Schauer wrote:
> >> @@ -347,7 +347,7 @@ sub create_ipconf_view {
> >>
> >>      my $get_device_desc = sub {
> >>  	my $iface = shift;
> >> -	return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> >> +	return "$iface->{name} - $iface->{mac} ($iface->{driver}) - $iface->{state}";
> >
> > I think it would be better to only show the state if the interface is
> > actually UP - thus drawing immediate attention to these list entries and
> > not completely overloading that dropdown.
> >
> > Maybe even append it as "(UP)" instead of by dash (as it does not really
> > identify the interface, but rather is a property of it).
> >
>
> Or, if the GTK component and available fonts support it we could also use
> unicode
I like the idea!

>
> UP   -> 🟢 https://unicode-explorer.com/c/1F7E2
A green circle would be problematic/non-optimal for people deuteranopia
(red-green color blindness). So maybe some other character/symbol
distinctly recognizable as "being connected, up" - but purely
color-coding things is not something that should be done IMO.

> DOWN -> ◯ (preferred, easier to differ for vision impaired people) or ⬤
>         https://unicode-explorer.com/c/25EF or https://unicode-explorer.com/c/2B24
Do we really need to explicitly mark non-UP interfaces? I'd think the
fact of it being not marked UP should provide enough context in this
situation.

>
> Would use less space and probably look better, if it works that is ^^

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Thomas Lamprecht]

Am 10/10/2023 um 14:56 schrieb Christoph Heiss:
> On Tue, Oct 10, 2023 at 01:55:43PM +0200, Thomas Lamprecht wrote:
>> UP   -> 🟢 https://unicode-explorer.com/c/1F7E2
> A green circle would be problematic/non-optimal for people deuteranopia
> (red-green color blindness). So maybe some other character/symbol
> distinctly recognizable as "being connected, up" - but purely
> color-coding things is not something that should be done IMO.

but those already get filled/not-filled as hint, i.e., this isn't just
purely color coding, that's why I explicitly used a *filled* green circle
for UP and for DOWN an *empty* one as preferred one due to vision
impairments like color blindness, like hinted ;-)

But sure, if you have a better idea for this just say so.
IMO the proposed one should be clear enough, as not only color (useful
for the majority of people) but also form is distinct, and I saw such
circles being used to indicate on/off or plugged/unplugged already in
a few UIs (such things are not easy to search for, so sorry, no concrete
example), but that doesn't mean there might be something better that is
still practical for both TUI and GUI.

> 
>> DOWN -> ◯ (preferred, easier to differ for vision impaired people) or ⬤
>>         https://unicode-explorer.com/c/25EF or https://unicode-explorer.com/c/2B24
> Do we really need to explicitly mark non-UP interfaces? I'd think the
> fact of it being not marked UP should provide enough context in this
> situation.

Yes, to give clear distinction to the UP state and make them more
distinct to each other, also to avoid text-alignment (visual symmetry)
problems (if the symbol prefixes the iface name, which I guesstimate
will look better).

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Filip Schauer]

The green circle is not displayed correctly by the PVE installer as it
does not have the corresponding emoji font package. However, the
suggested circles for DOWN are rendered correctly.

Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).

https://unicode-explorer.com/c/2B06
https://unicode-explorer.com/c/2B07

These arrows are also displayed correctly by the PVE installer.


On 10/10/2023 15:13, Thomas Lamprecht wrote:
> Am 10/10/2023 um 14:56 schrieb Christoph Heiss:
>> On Tue, Oct 10, 2023 at 01:55:43PM +0200, Thomas Lamprecht wrote:
>>> UP   -> 🟢 https://unicode-explorer.com/c/1F7E2
>> A green circle would be problematic/non-optimal for people deuteranopia
>> (red-green color blindness). So maybe some other character/symbol
>> distinctly recognizable as "being connected, up" - but purely
>> color-coding things is not something that should be done IMO.
> but those already get filled/not-filled as hint, i.e., this isn't just
> purely color coding, that's why I explicitly used a *filled* green circle
> for UP and for DOWN an *empty* one as preferred one due to vision
> impairments like color blindness, like hinted ;-)
>
> But sure, if you have a better idea for this just say so.
> IMO the proposed one should be clear enough, as not only color (useful
> for the majority of people) but also form is distinct, and I saw such
> circles being used to indicate on/off or plugged/unplugged already in
> a few UIs (such things are not easy to search for, so sorry, no concrete
> example), but that doesn't mean there might be something better that is
> still practical for both TUI and GUI.
>
>>> DOWN -> ◯ (preferred, easier to differ for vision impaired people) or ⬤
>>>          https://unicode-explorer.com/c/25EF or https://unicode-explorer.com/c/2B24
>> Do we really need to explicitly mark non-UP interfaces? I'd think the
>> fact of it being not marked UP should provide enough context in this
>> situation.
> Yes, to give clear distinction to the UP state and make them more
> distinct to each other, also to avoid text-alignment (visual symmetry)
> problems (if the symbol prefixes the iface name, which I guesstimate
> will look better).

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Thomas Lamprecht]

Am 11/10/2023 um 11:54 schrieb Filip Schauer:
> The green circle is not displayed correctly by the PVE installer as it
> does not have the corresponding emoji font package. However, the
> suggested circles for DOWN are rendered correctly.

The options I see (on top of that):

- we could just install a font package that ships it, e.g., the
  fonts-noto-color-emoji one, I mean 10.8 MB isn't negligible, but
  neither _that_ big..

- Use ◯ for down and ⬤ for up, and color them via CSS, or
  whatever is the easiest here for GTK combobox entries.

> Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).
> 
> https://unicode-explorer.com/c/2B06
> https://unicode-explorer.com/c/2B07
> 
> These arrows are also displayed correctly by the PVE installer.

I cannot 100% pin it down, but I do not really like arrows for
conveying that information even though the map to up/down directly,
but arrows are normally used for rather different things in UI
context (e.g., sorting, or resize handles), so IMO to overloaded
already.

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Filip Schauer]

On 11/10/2023 12:58, Thomas Lamprecht wrote:
> Am 11/10/2023 um 11:54 schrieb Filip Schauer:
>> The green circle is not displayed correctly by the PVE installer as it
>> does not have the corresponding emoji font package. However, the
>> suggested circles for DOWN are rendered correctly.
> The options I see (on top of that):
>
> - we could just install a font package that ships it, e.g., the
>    fonts-noto-color-emoji one, I mean 10.8 MB isn't negligible, but
>    neither _that_ big..
>
> - Use ◯ for down and ⬤ for up, and color them via CSS, or
>    whatever is the easiest here for GTK combobox entries.


Installing the font package for a single symbol seems a bit excessive.
However, changing the color of individual characters turns out to be
impossible with combo boxes. At least I haven't found a way to do this
with Gtk's limited CSS functionality, nor with override_color or a
custom cell renderer.

That leaves us with the font package.


>
>> Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).
>>
>> https://unicode-explorer.com/c/2B06
>> https://unicode-explorer.com/c/2B07
>>
>> These arrows are also displayed correctly by the PVE installer.
> I cannot 100% pin it down, but I do not really like arrows for
> conveying that information even though the map to up/down directly,
> but arrows are normally used for rather different things in UI
> context (e.g., sorting, or resize handles), so IMO to overloaded
> already.

Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

[Filip Schauer]

patch v2 available:
https://lists.proxmox.com/pipermail/pve-devel/2023-October/059450.html

On 11/10/2023 17:20, Filip Schauer wrote:
>
> On 11/10/2023 12:58, Thomas Lamprecht wrote:
>> Am 11/10/2023 um 11:54 schrieb Filip Schauer:
>>> The green circle is not displayed correctly by the PVE installer as it
>>> does not have the corresponding emoji font package. However, the
>>> suggested circles for DOWN are rendered correctly.
>> The options I see (on top of that):
>>
>> - we could just install a font package that ships it, e.g., the
>>    fonts-noto-color-emoji one, I mean 10.8 MB isn't negligible, but
>>    neither _that_ big..
>>
>> - Use ◯ for down and ⬤ for up, and color them via CSS, or
>>    whatever is the easiest here for GTK combobox entries.
>
>
> Installing the font package for a single symbol seems a bit excessive.
> However, changing the color of individual characters turns out to be
> impossible with combo boxes. At least I haven't found a way to do this
> with Gtk's limited CSS functionality, nor with override_color or a
> custom cell renderer.
>
> That leaves us with the font package.
>
>
>>
>>> Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).
>>>
>>> https://unicode-explorer.com/c/2B06
>>> https://unicode-explorer.com/c/2B07
>>>
>>> These arrows are also displayed correctly by the PVE installer.
>> I cannot 100% pin it down, but I do not really like arrows for
>> conveying that information even though the map to up/down directly,
>> but arrows are normally used for rather different things in UI
>> context (e.g., sorting, or resize handles), so IMO to overloaded
>> already.
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel