Skip to content
This repository has been archived by the owner on Oct 10, 2024. It is now read-only.

Refactor slider code, fix mouse wheel for kf5. #72

Open
wants to merge 5 commits into
base: kf5
Choose a base branch
from
Open

Refactor slider code, fix mouse wheel for kf5. #72

wants to merge 5 commits into from

Conversation

CatEricka
Copy link
Contributor

@CatEricka CatEricka commented Feb 25, 2024 •
edited
Loading

Alternative to #63 .

  1. Use signal onValueChanged instead of onMoved to simplify the code.
  2. Implement the scroll handling ourselves. This is a backport of https://invent.kde.org/plasma/libplasma/-/commit/aea3d4b131070d8388edf84c9c2a32f7b4203617

As for KDE Plasma 6, it needs to be tested. I don't have any machine with KDE Plasma 6 for now.

Related bug: https://bugreports.qt.io/browse/QTBUG-93081

@CatEricka
- Rename the debounce timer to make it more relevant.
fafe496 
- Make sure snapMode is set correctly.
davidhi7
davidhi7 reviewed Mar 4, 2024
View reviewed changes
plasmoid/contents/ui/main.qml Show resolved Hide resolved
plasmoid/contents/ui/main.qml Outdated Show resolved Hide resolved
plasmoid/contents/ui/main.qml
// Should also be locked during mouse wheel scrolling.
valuesLock = true
brightness = value
// Backport of https://invent.kde.org/plasma/libplasma/-/commit/aea3d4b131070d8388edf84c9c2a32f7b4203617
Copy link
Owner

Choose a reason for hiding this comment

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

If I understood this correctly, this code doesn't need to be implemented for the KF6 version because it's built into PlasmaComponents.Slider?

Copy link
Contributor Author

@CatEricka CatEricka Mar 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe? It depends on when the distro updates KF6 versions.

@CatEricka
Copy link
Contributor Author

Sorry for the late reply, I will modify the code you mentioned as soon as possible and test it.

@CatEricka
Copy link
Contributor Author

CatEricka commented Mar 10, 2024 •
edited
Loading

I'm not convinced this is no longer needed. IIRC the main reason to implement it was that if you drag the slider and new polling data is received, the slider is reset to its original value.

After thinking about it for a weekend, I thought of a way to fix this issue: let the backend return messages with timestamps, and let plasmoid ignore the old dirty data.

Edited:

This may be too complex.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davidhi7 davidhi7 davidhi7 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CatEricka @davidhi7