[pve-devel] [PATCH manager 1/3] headerbar: use same height and padding for product name as PBS and PMG

[pve-devel] [PATCH manager 1/3] headerbar: use same height and padding for product name as PBS and PMG

[Lukas Wagner]

This makes the header bar look a bit less cramped and ensures consistency
if web interfaces are used side-by-side, e.g. on an ultrawide screen.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Disadvantage: We loose 8px of vertical space. However, I really think
    this is negligible.

 www/manager6/Workspace.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 18d574b7..006e7f29 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -322,6 +322,7 @@ Ext.define('PVE.StdWorkspace', {
 		    },
 		    border: false,
 		    margin: '2 0 2 5',
+		    height: 38,
 		    items: [
 			{
 			    xtype: 'proxmoxlogo',
@@ -334,6 +335,7 @@ Ext.define('PVE.StdWorkspace', {
 				'font-size': '14px',
 				'line-height': '18px',
 			    },
+			    padding: '0 0 0 5',
 			},
 			{
 			    xtype: 'pveGlobalSearchField',
-- 
2.39.2

[pve-devel] [PATCH manager 2/3] headerbar: show shortcut for search in emptyText

[Lukas Wagner]

The shortcut is not really documented anywhere, so I think it make it a
bit more obvious to the user.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/form/GlobalSearchField.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/form/GlobalSearchField.js b/www/manager6/form/GlobalSearchField.js
index c009ac8b..8e5e13c0 100644
--- a/www/manager6/form/GlobalSearchField.js
+++ b/www/manager6/form/GlobalSearchField.js
@@ -8,7 +8,7 @@ Ext.define('PVE.form.GlobalSearchField', {
     extend: 'Ext.form.field.Text',
     alias: 'widget.pveGlobalSearchField',
 
-    emptyText: gettext('Search'),
+    emptyText: gettext('Search (Ctrl-Shift-F)'),
     enableKeyEvents: true,
     selectOnFocus: true,
     padding: '0 5 0 5',
-- 
2.39.2

Re: [pve-devel] [PATCH manager 2/3] headerbar: show shortcut for search in emptyText

[Thomas Lamprecht]

On 21/07/2023 11:39, Lukas Wagner wrote:
> The shortcut is not really documented anywhere, so I think it make it a
> bit more obvious to the user.

style not: commit message should wrap text at 70 character columns.
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages

> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  www/manager6/form/GlobalSearchField.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/www/manager6/form/GlobalSearchField.js b/www/manager6/form/GlobalSearchField.js
> index c009ac8b..8e5e13c0 100644
> --- a/www/manager6/form/GlobalSearchField.js
> +++ b/www/manager6/form/GlobalSearchField.js
> @@ -8,7 +8,7 @@ Ext.define('PVE.form.GlobalSearchField', {
>      extend: 'Ext.form.field.Text',
>      alias: 'widget.pveGlobalSearchField',
>  
> -    emptyText: gettext('Search'),
> +    emptyText: gettext('Search (Ctrl-Shift-F)'),

not true for MacOS though, maybe check navigator.platform for /mac/i and
depending on that use CTRL or ⌘ 

Semi-related, is the aria-keyshortcuts attribute actually set here?

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-keyshortcuts

Re: [pve-devel] [PATCH manager 2/3] headerbar: show shortcut for search in emptyText

[Lukas Wagner]

On 7/21/23 15:31, Thomas Lamprecht wrote:
> On 21/07/2023 11:39, Lukas Wagner wrote:
>> The shortcut is not really documented anywhere, so I think it make it a
>> bit more obvious to the user.
> 
> style not: commit message should wrap text at 70 character columns.
> https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
> 
Yes, I'm sorry. Usually neovim wraps automatically, but that does not work
when I edit lines afterwards. Really got to set up a color column for
commit messages.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>   www/manager6/form/GlobalSearchField.js | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/www/manager6/form/GlobalSearchField.js b/www/manager6/form/GlobalSearchField.js
>> index c009ac8b..8e5e13c0 100644
>> --- a/www/manager6/form/GlobalSearchField.js
>> +++ b/www/manager6/form/GlobalSearchField.js
>> @@ -8,7 +8,7 @@ Ext.define('PVE.form.GlobalSearchField', {
>>       extend: 'Ext.form.field.Text',
>>       alias: 'widget.pveGlobalSearchField',
>>   
>> -    emptyText: gettext('Search'),
>> +    emptyText: gettext('Search (Ctrl-Shift-F)'),
> 
> not true for MacOS though, maybe check navigator.platform for /mac/i and
> depending on that use CTRL or ⌘

Very good point, I did not think about that.
Macs do have a control key and that ctrl key seems to have the same keycode [1], so
the label would also be correct on a Mac, and the shortcut *should* work.

However, it would of course be more idiomatic to use Cmd-Shift-F, because
browsers usually use Cmd-F for search on macOS.
So in other words, we should adapt the label AND change the code that sets
up the keyboard shortcut.
Will look into that, just need to find a way how to test this properly ;)
> 
> Semi-related, is the aria-keyshortcuts attribute actually set here?
> 
> https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-keyshortcuts

Judging from the web inspector, this attribute does not seem to be set.
I'll look into that.


[1] https://stackoverflow.com/questions/47117199/control-key-in-web-browser-on-mac

-- 
- Lukas

[pve-devel] [PATCH manager 3/3] headerbar: center search box, make it adapt to the width of the browser

[Lukas Wagner]

Also adding a minWidth/maxWidth so that it doesn't become comically
large or small. The minimum size is roughly the same size as the
search bar was before. The maxmium size is the same size as the
results grid, making them align nicely.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Played around a bit and found the centered search visually quite pleasing.
    A few other people in the office also liked it.

 www/manager6/Workspace.js              | 7 +++++--
 www/manager6/form/GlobalSearchField.js | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 006e7f29..b8901912 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -337,12 +337,16 @@ Ext.define('PVE.StdWorkspace', {
 			    },
 			    padding: '0 0 0 5',
 			},
+			{
+			    flex: 0.5,
+			},
 			{
 			    xtype: 'pveGlobalSearchField',
 			    tree: rtree,
+			    flex: 1.60,
 			},
 			{
-			    flex: 1,
+			    flex: 0.5,
 			},
 			{
 			    xtype: 'proxmoxHelpButton',
@@ -530,4 +534,3 @@ Ext.define('PVE.StdWorkspace', {
 	});
     },
 });
-
diff --git a/www/manager6/form/GlobalSearchField.js b/www/manager6/form/GlobalSearchField.js
index 8e5e13c0..ee352cdc 100644
--- a/www/manager6/form/GlobalSearchField.js
+++ b/www/manager6/form/GlobalSearchField.js
@@ -12,6 +12,8 @@ Ext.define('PVE.form.GlobalSearchField', {
     enableKeyEvents: true,
     selectOnFocus: true,
     padding: '0 5 0 5',
+    minWidth: 200,
+    maxWidth: 600,
 
     grid: {
 	xtype: 'gridpanel',
-- 
2.39.2

Re: [pve-devel] [PATCH manager 3/3] headerbar: center search box, make it adapt to the width of the browser

[Thomas Lamprecht]

unsure about this, need to think a bit and as a few other things
are pending and some vacation too I'd say let's revisit in September
(i.e., ping me then please) if I didn't get around to it.

On 21/07/2023 11:39, Lukas Wagner wrote:
> Also adding a minWidth/maxWidth so that it doesn't become comically
> large or small. The minimum size is roughly the same size as the

you can use numbers here, IMO 168 px and 200 px is not roughly the
same ^^

> search bar was before. The maxmium size is the same size as the
> results grid, making them align nicely.

Would be centering the search result grid then be good to? Also,
making it bigger if viewport size allows it, e.g. due to having more
stuff like the tags shown in there nowadays.

Also, we predominantly use integer flex values, besides consistency
one IMO has a better ration understanding there.

Re: [pve-devel] [PATCH manager 3/3] headerbar: center search box, make it adapt to the width of the browser

[Lukas Wagner]

On 7/21/23 15:54, Thomas Lamprecht wrote:
> unsure about this, need to think a bit and as a few other things
> are pending and some vacation too I'd say let's revisit in September
> (i.e., ping me then please) if I didn't get around to it.
> 

Sure. Will do. I just experimented a bit and a few people quite liked it, so
I thought I'd give it a shot.

> On 21/07/2023 11:39, Lukas Wagner wrote:
>> Also adding a minWidth/maxWidth so that it doesn't become comically
>> large or small. The minimum size is roughly the same size as the
> 
> you can use numbers here, IMO 168 px and 200 px is not roughly the
> same ^^
> 
Well, I would still argue that for a user who just takes a glance and
is not actually counting pixels it's pretty similar ;)

>> search bar was before. The maxmium size is the same size as the
>> results grid, making them align nicely.
> 
> Would be centering the search result grid then be good to? Also,
> making it bigger if viewport size allows it, e.g. due to having more
> stuff like the tags shown in there nowadays.
> 
> Also, we predominantly use integer flex values, besides consistency
> one IMO has a better ration understanding there.

Okay, now I'm really glad that I resisted the temptation to use 1.61
(golden ratio), because that is actually where the 1.6 came from ;)
But noted, thanks.

-- 
- Lukas

Re: [pve-devel] [PATCH manager 1/3] headerbar: use same height and padding for product name as PBS and PMG

[Thomas Lamprecht]

On 21/07/2023 11:39, Lukas Wagner wrote:
> This makes the header bar look a bit less cramped and ensures consistency
> if web interfaces are used side-by-side, e.g. on an ultrawide screen.> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 
> Notes:
>     Disadvantage: We loose 8px of vertical space. However, I really think
>     this is negligible.

well we could also meed in the middle, if having 32, 34 or 36 px is
enough to make it "less cramped"? As I don't like ranking subjective
design tuning over actual UX, and even if just 8 px, it adds up
(especially when then uses this change to argue adding some white space
padding elsewhere).

Also, Proxmox Backup Server, Proxmox Mail Gateway have a relatively
different base UI structure too, and we normally try only hard to keep
things consistent in the same product, not that it hurts on its own to
do so also between products, but that shouldn't be the sole argument.

Still some polishing is good, and all in all this can be fine, but as
said I'd check for a trade-off.

Re: [pve-devel] [PATCH manager 1/3] headerbar: use same height and padding for product name as PBS and PMG

[Lukas Wagner]

On 7/21/23 16:04, Thomas Lamprecht wrote:
> well we could also meed in the middle, if having 32, 34 or 36 px is
> enough to make it "less cramped"? 
Yeah, the main reason for 38px was the consistency to other products.

Now that I play around a bit with different heights, I feel like the
major visual improvement (I know, this is subjective) comes from
the added horizontal padding between the logo and the product name (5px),
as well as the centered search. I'd be happy to settle on 30px it that
makes it any easier to sell the centered, larger search box. ;)


-- 
- Lukas