search

lxqt / lxqt-qtplugin

LXQt Qt platform integration plugin
https://lxqt.github.io
GNU Lesser General Public License v2.1
24 stars 14 forks source link

Enable `QPlatformTheme::HoverEffect` #82

Closed q234rty closed 2 months ago

q234rty commented 3 months ago

QPlatformTheme::HoverEffect is defined in https://invent.kde.org/qt/qt/qtbase/-/blob/6.6.1/src/gui/kernel/qplatformtheme.h#L264-L274.

According to https://invent.kde.org/qt/qt/qtbase/-/blob/6.6.1/src/gui/kernel/qstylehints.cpp?ref_type=heads#L501-L517 and https://invent.kde.org/qt/qt/qtbase/-/blob/dev/src/gui/platform/unix/qgenericunixthemes.cpp#L503-L504, this should be enabled by default on desktop platforms.

However, currently QPlatformTheme::themeHint is used as the fallback, bypassing this logic in Qt.

It might be better to use QGenericUnixTheme as a fallback instead (note that this will also "fix" #78). Or we can just enable it by default ourselves if that is not desirable.

tsujan commented 3 months ago

@palinek, @luis-pereira, @yan12125?

tsujan commented 3 months ago

QPlatformTheme::HoverEffect is defined in https://invent.kde.org/qt/qt/qtbase/-/blob/6.6.1/src/gui/kernel/qplatformtheme.h#L264-L274.

It also exists in 6.6.0, which is our minimum required version. So, we can cover it.

IMO, it should be enabled in LXQt. Any objection?

stefonarch commented 3 months ago

It also exists in 6.0.0, which is our minimum required version.

Typo I guess, 6.6.0. No objection.

tsujan commented 3 months ago

Typo I guess, 6.6.0.

Yes. Corrected it.

palinek commented 2 months ago

IMO, it should be enabled in LXQt.

Do you suggest to make UI effects to be configurable?

tsujan commented 2 months ago

Do you suggest to make UI effects to be configurable?

No. I suggest what is recommended:

    case UiEffects:
        return QVariant(int(HoverEffect));
palinek commented 2 months ago

Why just that one, when there are multiple effects available?

    enum UiEffect
    {
        GeneralUiEffect = 0x1,
        AnimateMenuUiEffect = 0x2,
        FadeMenuUiEffect = 0x4,
        AnimateComboUiEffect = 0x8,
        AnimateTooltipUiEffect = 0x10,
        FadeTooltipUiEffect = 0x20,
        AnimateToolBoxUiEffect = 0x40,
        HoverEffect = 0x80
    };
tsujan commented 2 months ago

According to https://invent.kde.org/qt/qt/qtbase/-/blob/6.6.1/src/gui/kernel/qstylehints.cpp?ref_type=heads#L501-L517, "This is the standard behavior on desktop platforms with a mouse pointer...".

I see no reason to confuse users with more options, but I'm not against them; just don't suggest them.

palinek commented 2 months ago

Then w/o any new configuration, I agree to just use the same as in the generic unix theme.

tsujan commented 2 months ago

A PR, please?

palinek commented 2 months ago

A PR, please?

86