Center systray vertically

[FeZoli]

If the user sets the systay size with systray:set_base_size(...), then this change centers the systray vertically.

[FeZoli]

Hi There,

I'm not a lua programmer, just modified the code for myself, and thought it might be useful.

Cheers

[Aire-One]

I'm not fundamentally opposed to this change. But it would be a behavior change I think we don't want to make now.

Anyway, AFAICT, icons in the systray are all supposed to be squares of the same size. So whatever the base_size is, icons are always equally placed in. Hence there is no need to center them. Am I missing something?

[Aire-One]

I think base_size is nil if the user hasn't manually changed it before.

[Aire-One]

The CI confirms that

2023-10-02T18:16:23.2795466Z 56 tests finished.
2023-10-02T18:16:23.2796918Z There were 1 errors:
2023-10-02T18:16:23.2806984Z  - /home/runner/work/awesome/awesome/tests/test-systray.lua: 2023-10-02 18:16:16 E: awesome: Error during a protected call: lib/wibox/widget/systray.lua:69: attempt to perform arithmetic on a nil value (upvalue 'base_size')

[actionless]

@Aire-One i'm not sure if it has something to do with DPI support across GUI toolkits, or otherwise related to systray implementation in the apps - but the problem indeed real, some apps getting smaller icons than others

[actionless]

oops, wrong thread, i thought i'm replying to #3860 (review)

[FeZoli]

I'm not fundamentally opposed to this change. But it would be a behavior change I think we don't want to make now.

Anyway, AFAICT, icons in the systray are all supposed to be squares of the same size. So whatever the base_size is, icons are always equally placed in. Hence there is no need to center them. Am I missing something?

This is my reasoning:

• I have a bar with 25 px height
• I have a systray 3 apps in it. Unfortunately they have different sizes for whatever reason: MS Teams 25px, NextCloud 25px, nm-applet 18 px. Rather ugly
• So I have set the systray base size to 18 px
• Now the systray is aligned to the top of the bar. I wanted to be centered vertically.

That's why I've created this change.
Cheers.

[codecov]

Codecov Report

Attention: Patch coverage is 45.00000% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 91.21%. Comparing base (e6f5c79) to head (5017aca).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3860      +/-   ##
==========================================
- Coverage   91.22%   91.21%   -0.02%     
==========================================
  Files         927      927              
  Lines       59493    59512      +19     
==========================================
+ Hits        54275    54282       +7     
- Misses       5218     5230      +12     

Flag	Coverage Δ
gcov	91.21% <45.00%> (-0.02%)	⬇️
luacov	93.85% <45.00%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
lib/wibox/widget/systray.lua	83.87% <45.00%> (-7.56%)	⬇️

... and 2 files with indirect coverage changes

[actionless]

i'd rather make it optional

[FeZoli]

i'd rather make it optional

Please take a look at my screenshots:

1. Original systray, icons with different sizes:
   http://share.minux.hu/Screenshot_20231009_074423_orig.png

2. Original systray after setting the systray height to 18px. Icons are aligned top now. See, they are in offset compared to the text:
   http://share.minux.hu/Screenshot_20231009_074338_offset.png

3. Patched version. Icons are now center aligned. I think it should be the default.
   http://share.minux.hu/Screenshot_20231009_074118_patched.png

Cheers,
FeZ

[unai-ndz]

I get "The provided host name is not valid for this server." when trying to access those images. And maybe use https?

[FeZoli]

Hi,

I have uploaded the screenshots into google drive:

https://drive.google.com/drive/folders/1dDN8Td_QqbTvBC1gTbEjiEto8F1T5gaI?usp=drive_link

I recap the reasoning:

1. Original systray, icons with different sizes:
   Screenshot_20231009_074423_orig.png

2. Original systray after setting the systray height to 18px. Icons are aligned top now. See, they are in offset compared to the text:
Screenshot_20231009_074338_offset.png

3. Patched version. Icons are now center aligned. I think it should be the default.
Screenshot_20231009_074118_patched.png

Hope it works now.

Cheers,
FeZ

[Aire-One]

As @actionless suggested, let's make this change optional.

[sclu1034]

I'm not fundamentally opposed to this change. But it would be a behavior change I think we don't want to make now.
Anyway, AFAICT, icons in the systray are all supposed to be squares of the same size. So whatever the base_size is, icons are always equally placed in. Hence there is no need to center them. Am I missing something?

This is my reasoning:

* I have a bar with 25 px height

* I have a systray 3 apps in it. Unfortunately they have different sizes for whatever reason: MS Teams 25px, NextCloud 25px, nm-applet 18 px. Rather ugly

* So I have set the systray base size to 18 px

* Now the systray is aligned to the top of the bar. I wanted to be centered vertically.

That's why I've created this change. Cheers.

For this specific use case, where the icons are all the same size, wibox.container.place is already sufficient to align the systray in its parent (unless the systray widget doesn't shrink to the icon size, which would be an entirely different issue to fix).
Alignment internally to the systray widget only makes sense for a use case where the icons are left with their non-matching sizes.

And even for that case, it shouldn't just be "centered vs not centered". It should be the same API as https://awesomewm.org/apidoc/widget_containers/wibox.container.place.html#valign.

[actionless]

good point about "porting" full valign property's logic instead of implementing just center/non-center 👍

[FeZoli]

For this specific use case, where the icons are all the same size, wibox.container.place is already sufficient to align the systray in its parent (unless the systray widget doesn't shrink to the icon size, which would be an entirely different issue to fix). Alignment internally to the systray widget only makes sense for a use case where the icons are left with their non-matching sizes.

I have set

wibox.container.place.valign = "center"

in my theme.lua

Now the entire wibar has changed, and some properties are not set up correctly (font is not set, layout icon is missing, etc.). No error message, though. See the screenshot:

https://drive.google.com/file/d/1AnXgfUZov4fvsrtfIclGbNKlMCGIqU4l/view?usp=drive_link

What did I do wrong?

Thanks.