Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

isf63
Copy link
Contributor

@isf63 isf63 commented Apr 12, 2024

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.

Fixes an inconsistency in themes when they expect an expanding size policy,
which is what most plugins have set. Some themes that do this: KDE-Plasma,
Ambiance, Kvantum. Further stylesheet changes in some themes will be needed
to match themes exactly.
@isf63
Copy link
Contributor Author

isf63 commented May 22, 2024

To illustrate the issue: (in Ambiance theme)

vid.mp4

@isf63
Copy link
Contributor Author

isf63 commented Aug 25, 2024

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

@tsujan
Copy link
Member

tsujan commented Aug 25, 2024

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
Copy link
Contributor Author

isf63 commented Aug 25, 2024

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
Copy link
Member

tsujan commented Aug 25, 2024

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
Copy link
Member

tsujan commented Aug 25, 2024

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

@isf63
Copy link
Contributor Author

isf63 commented Aug 25, 2024

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
Copy link
Member

tsujan commented Aug 25, 2024

I'll check it tomorrow.

Copy link
Member

@tsujan tsujan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR has no problem.

However,

  • It's rather a matter of taste. Contrary to the PR comment, there's no issue to fix.
  • We don't and can't fix problems of LXQt themes in the code of lxqt-panel. For example, the KDE-Plasma theme lacks lines like these:
#ColorPickerPickerButton,
#ColorPickerColorButton {
    margin: 3px;
    border-top: 3px solid transparent;
    color: #313639;
}

#ColorPickerPickerButton:hover,
#ColorPickerColorButton:hover {
    border-top: 3px solid rgba(30, 145, 255, 50%);
    color: rgba(54, 54, 54, 100%);
}

@tsujan
Copy link
Member

tsujan commented Aug 26, 2024

@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
Copy link
Member

It's fine for me.

@tsujan tsujan merged commit 626fede into lxqt:master Aug 26, 2024
1 check failed
tsujan pushed a commit that referenced this pull request Aug 27, 2024
* Initial backend plugin infrastructure

* Update licenses

* lxqttaskbartypes.h: fix ShowOnAll desktops flag value

* Fix backend load logic: do not load zero score backends

- Fix X11 backend to return zero score on non-X11 platforms

* LXQtPanelApplication: always find best backend at startup

If preferred backend is set try it first.
Do not set preferred backend automatically. It must be user choice.

* Panel backends: pass string argument for score calculation

- Split XDG_CURRENT_DESKTOP
- Skip LXQTPANEL_PLUGIN_PATH if empty

* Backends: change name scheme

libwmbackend_<platform>.so

* LXQtPanelApplication: only consider plugins with valid names

* LXQtPanelApplication: fix empty backend message

* TaskBar: add experimental KWin Wayland backend

NOTE: works only on KWin

- Choose backend at runtime
- Windows filter logic is re-evaluated on window property changes

LXQtTaskBarPlasmaWindowManagment: implement showDesktop()

LXQtTaskbarWaylandBackend: do not show transient windows

LXQtTaskBarPlasmaWindowManagment: fix destructor TODO

TODO: is this correct?
Seems to call wl_proxy_destroy underneath

LXQtPanel: basic virtual desktop support on Plasma Wayland

Add desktop file to be recognized by KWin Wayland

NOTE: absolute path is needed inside .desktop file for this to work
      use CMake to get it.

- Prevent double dekstop file installed in autostart

LXQtTaskbarWaylandBackend: return only accepted windows

- reloadWindows() force removal and readding of windows

This fixes changing windows grouping settings and adding taskbar plugin
AFTER panel is started.
Both situations resulted in empty taskbar previously

LXQtTaskbarWaylandBackend: fix workspace logic

LXQtTaskbarWaylandBackend: fix workspace removal logic

LXQtTaskbarWaylandBackend: implement moving window to virtual desktop
workspace

LXQtPlasmaWaylandWorkspaceInfo: fix signedness comparison

CMake: move panel WM backends to separate libraries

LXQtTaskbarWaylandBackend: possibly fix crash on showDesktop for non-
KWin

Update license headers

LXQtTaskbarWaylandBackend: add dummy setDesktopLayout()

Implement LXQtWMBackendKWinWaylandLibrary

- Add Desktop Environment detection

* LXQtPanel: workaround KAcceleratorManager changing button text FIXME TODO

TODO: is this correct approach?

* ColorPicker: use XDG Desktop Portal on Wayland TODO

TODO: show error message when not supported

* Hide lxqt-panel application from applications menu

- Add NoDisplay=true to .desktop file

CMake: rename autostart desktop variable

* LXQtWMBackend_KWinWayland: announce DesktopSwitch support

* LXQtWMBackend_KWinWayland: fix minimize on click not working

* Backend detection and Wlroots backend

* Fix autodetection on user choice failure

* wmbackend_kwin_wayland: Compare against kwin_wayland.

* Iterate through all parts of the XDG_CURRENT_DESKTOP. Add river to wlroots

* Set size policy in plugin-backlight and plugin-colorpicker (#2049)

Fixes an inconsistency in themes when they expect an expanding size policy,
which is what most plugins have set. Some themes that do this: KDE-Plasma,
Ambiance, Kvantum. Further stylesheet changes in some themes will be needed
to match themes exactly.

* Weblate commit (#2085)

Translate-URL: https://translate.lxqt-project.org/projects/lxqt-panel/plugin-fancymenu/da/
Translate-URL: https://translate.lxqt-project.org/projects/lxqt-panel/plugin-kbindindicator/ko/
Translation: LXQt Panel/plugin-fancymenu
Translation: LXQt Panel/plugin-kbindicator

Co-authored-by: Peter  Jespersen <[email protected]>
Co-authored-by: 이정희 <[email protected]>

* Initial backend plugin infrastructure

* Update licenses

* lxqttaskbartypes.h: fix ShowOnAll desktops flag value

* Fix backend load logic: do not load zero score backends

- Fix X11 backend to return zero score on non-X11 platforms

* LXQtPanelApplication: always find best backend at startup

If preferred backend is set try it first.
Do not set preferred backend automatically. It must be user choice.

* Panel backends: pass string argument for score calculation

- Split XDG_CURRENT_DESKTOP
- Skip LXQTPANEL_PLUGIN_PATH if empty

* Backends: change name scheme

libwmbackend_<platform>.so

* LXQtPanelApplication: only consider plugins with valid names

* LXQtPanelApplication: fix empty backend message

* TaskBar: add experimental KWin Wayland backend

NOTE: works only on KWin

- Choose backend at runtime
- Windows filter logic is re-evaluated on window property changes

LXQtTaskBarPlasmaWindowManagment: implement showDesktop()

LXQtTaskbarWaylandBackend: do not show transient windows

LXQtTaskBarPlasmaWindowManagment: fix destructor TODO

TODO: is this correct?
Seems to call wl_proxy_destroy underneath

LXQtPanel: basic virtual desktop support on Plasma Wayland

Add desktop file to be recognized by KWin Wayland

NOTE: absolute path is needed inside .desktop file for this to work
      use CMake to get it.

- Prevent double dekstop file installed in autostart

LXQtTaskbarWaylandBackend: return only accepted windows

- reloadWindows() force removal and readding of windows

This fixes changing windows grouping settings and adding taskbar plugin
AFTER panel is started.
Both situations resulted in empty taskbar previously

LXQtTaskbarWaylandBackend: fix workspace logic

LXQtTaskbarWaylandBackend: fix workspace removal logic

LXQtTaskbarWaylandBackend: implement moving window to virtual desktop
workspace

LXQtPlasmaWaylandWorkspaceInfo: fix signedness comparison

CMake: move panel WM backends to separate libraries

LXQtTaskbarWaylandBackend: possibly fix crash on showDesktop for non-
KWin

Update license headers

LXQtTaskbarWaylandBackend: add dummy setDesktopLayout()

Implement LXQtWMBackendKWinWaylandLibrary

- Add Desktop Environment detection

* LXQtPanel: workaround KAcceleratorManager changing button text FIXME TODO

TODO: is this correct approach?

* ColorPicker: use XDG Desktop Portal on Wayland TODO

TODO: show error message when not supported

* Hide lxqt-panel application from applications menu

- Add NoDisplay=true to .desktop file

CMake: rename autostart desktop variable

* LXQtWMBackend_KWinWayland: announce DesktopSwitch support

* LXQtWMBackend_KWinWayland: fix minimize on click not working

* Backend detection and Wlroots backend

* Fix autodetection on user choice failure

* wmbackend_kwin_wayland: Compare against kwin_wayland.

* Iterate through all parts of the XDG_CURRENT_DESKTOP. Add river to wlroots

* Rever changes to color-picker

* Revert changes to color-picker, attempt 2

---------

Co-authored-by: Filippo Gentile <[email protected]>
Co-authored-by: isf63 <[email protected]>
Co-authored-by: LXQtBot <[email protected]>
Co-authored-by: Peter  Jespersen <[email protected]>
Co-authored-by: 이정희 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants