Center systray vertically #3860

FeZoli · 2023-10-02T06:25:52Z

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

FeZoli · 2023-10-02T06:28:31Z

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 · 2023-10-02T22:10:26Z

lib/wibox/widget/systray.lua

@@ -66,6 +66,7 @@ function systray:draw(context, cr, width, height)
    local cols = math.ceil(num_entries / rows)
    local bg = beautiful.bg_systray or beautiful.bg_normal or "#000000"
    local spacing = beautiful.systray_icon_spacing or 0
+    local y_offset = ((height - base_size) / 2) - 1


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

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')

@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

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

FeZoli · 2023-10-03T05:26:09Z

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 · 2023-10-03T10:58:13Z

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 · 2023-10-05T17:53:03Z

i'd rather make it optional

FeZoli · 2023-10-09T05:51:36Z

i'd rather make it optional

Please take a look at my screenshots:

Original systray, icons with different sizes:
http://share.minux.hu/Screenshot_20231009_074423_orig.png
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
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 · 2023-10-24T19:04:28Z

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

FeZoli · 2023-10-30T06:38:10Z

Hi,

I have uploaded the screenshots into google drive:

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

I recap the reasoning:

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.

lib/wibox/widget/systray.lua

sclu1034 · 2023-11-13T08:25:14Z

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 · 2023-11-19T01:46:17Z

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

FeZoli · 2023-12-26T09:48:09Z

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.

Merge from central.

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

Removed trailing whitespace. Co-authored-by: Aire-One <aireone@aireone.xyz>

Removed unnecessary whitespces.

Aire-One reviewed Oct 2, 2023

View reviewed changes

Aire-One requested changes Nov 12, 2023

View reviewed changes

lib/wibox/widget/systray.lua Outdated Show resolved Hide resolved

FeZoli and others added 5 commits February 4, 2024 06:56

Merge pull request #1 from awesomeWM/master

ba69dfe

Merge from central.

Center systray vertically

fa432e7

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

Fixed error when base_size is not set

9301c49

Update lib/wibox/widget/systray.lua

a554498

Removed trailing whitespace. Co-authored-by: Aire-One <aireone@aireone.xyz>

Applied common API to determine vertical alignment.

ead4fc3

FeZoli force-pushed the FeZoli-patch-1 branch from 66ec890 to ead4fc3 Compare February 4, 2024 09:52

Removed unused variable.

5017aca

Removed unnecessary whitespces.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Center systray vertically #3860

Center systray vertically #3860

FeZoli commented Oct 2, 2023

FeZoli commented Oct 2, 2023

Aire-One left a comment

Aire-One Oct 2, 2023

Aire-One Oct 2, 2023

actionless Oct 24, 2023 •

edited

Loading

actionless Oct 24, 2023

FeZoli commented Oct 3, 2023

codecov bot commented Oct 3, 2023 •

edited

Loading

actionless commented Oct 5, 2023

FeZoli commented Oct 9, 2023

unai-ndz commented Oct 24, 2023

FeZoli commented Oct 30, 2023 •

edited

Loading

Aire-One left a comment

sclu1034 commented Nov 13, 2023 •

edited

Loading

actionless commented Nov 19, 2023 •

edited

Loading

FeZoli commented Dec 26, 2023

Center systray vertically #3860

Are you sure you want to change the base?

Center systray vertically #3860

Conversation

FeZoli commented Oct 2, 2023

FeZoli commented Oct 2, 2023

Aire-One left a comment

Choose a reason for hiding this comment

Aire-One Oct 2, 2023

Choose a reason for hiding this comment

Aire-One Oct 2, 2023

Choose a reason for hiding this comment

actionless Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

actionless Oct 24, 2023

Choose a reason for hiding this comment

FeZoli commented Oct 3, 2023

codecov bot commented Oct 3, 2023 • edited Loading

Codecov Report

actionless commented Oct 5, 2023

FeZoli commented Oct 9, 2023

unai-ndz commented Oct 24, 2023

FeZoli commented Oct 30, 2023 • edited Loading

Aire-One left a comment

Choose a reason for hiding this comment

sclu1034 commented Nov 13, 2023 • edited Loading

actionless commented Nov 19, 2023 • edited Loading

FeZoli commented Dec 26, 2023

actionless Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading

FeZoli commented Oct 30, 2023 •

edited

Loading

sclu1034 commented Nov 13, 2023 •

edited

Loading

actionless commented Nov 19, 2023 •

edited

Loading