search

lxqt / lxqt-panel

The LXQt desktop panel
https://lxqt-project.org
GNU Lesser General Public License v2.1
194 stars 135 forks source link

Set size policy in plugin-backlight and plugin-colorpicker #2049

Closed isf63 closed 3 months ago

isf63 commented 7 months ago

Fixes an inconsistency in themes when they expect an expanding size policy, which is what most plugins have set. Some themes with incorrect styling due to this: KDE-Plasma, Ambiance, Kvantum.

isf63 commented 6 months ago

To illustrate the issue: (in Ambiance theme)

https://github.com/lxqt/lxqt-panel/assets/121320947/67a47316-57e0-4161-aeec-a110218231be

isf63 commented 3 months ago

@palinek @tsujan - would anyone mind taking a look at this? I think it's fairly straightforward.

tsujan commented 3 months ago

As for me, since I don't use those plugins or different LXQt themes, frankly I don't know what it's about.

A potential reviewer might be more motivated if the PR maker links his PR to an issue that clearly describes a problem.

isf63 commented 3 months ago

I can describe the (minor) issue here:

  1. Almost all panel plugins have set their QSizePolicy to Expanding.
  2. Many themes assume that property and incorporate it into their styling. Background style and mouse bounding boxes are some aspects that are affected.
  3. Thus the missing property on two plugins appears to be a minor bug.

To reproduce, set LXQt theme to (KDE-Plasma|Kvantum|Ambiance). The hover/active/over-bar states are not displayed correctly.

tsujan commented 3 months ago

Thanks for the description!

The hover/active/over-bar states are not displayed

What does that have to do with the size policy? I mean the mere fact that changing the size policy to "expanding" restores the hover state isn't enough. We should make sure that there isn't another reason, and also that the new size policy won't have a side effect.

tsujan commented 3 months ago

Also, what happens if you remove those three size policies in colorpicker and leave them to be Qt's default?

isf63 commented 3 months ago

hover/active/over-bar states are not displayed correctly

remove those three size policies in colorpicker

I believe it defaults to QSizePolicy::Fixed, which causes the bounding box to be minimized. The hover/active states are affected indirectly by not being styled consistently with the rest of the plugins. Over-bars, the same thing, but it may be necessary to have a gap between panel height and icon size to see it.

tsujan commented 3 months ago

I'll check it tomorrow.

tsujan commented 3 months ago

@stefonarch If you have no problem with this change, I'm going to merge it. It makes the buttons of those plugins expand to the available vertical space with horizontal panels (and similarly with vertical ones).

stefonarch commented 3 months ago

It's fine for me.