Add dedicated dragging state for slider widget #3533
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3533 +/- ##
========================================
Coverage 90.66% 90.67%
========================================
Files 852 855 +3
Lines 54440 54843 +403
========================================
+ Hits 49358 49727 +369
- Misses 5082 5116 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Thanks for this. Too bad testing this would be rather non trivial. If you want to do it, you could copy/paste |
When trying to hook up the slider to an external value, one would usually have two data streams: - `property::value` -> set external value - external value changed -> `.value = new_value` The problem is that without manual intervention, these two streams form a loop, as setting `.value` also emits `property::value`. The new set of signals is disconnected from the `value` property and its signal and allows for more fine grained inspection of the dragging state. Signed-off-by: Lucas Schwiderski <lucas@lschwiderski.de>
sclu1034
force-pushed
the
feature/slider
branch
2 times, most recently
from
d632a67
to
6343c5f
Compare
|
The test runs fine when executed solo ( |
|
Usually, you have to check for completion in each step or I would also appreciate not adding a new hard dependency here. Even |
sclu1034
force-pushed
the
feature/slider
branch
from
6343c5f
to
049aeef
Compare
|
Replaced the |
|
The failure is strange. I am not so sure. Maybe adding a couple empty steps after the wibox creation might help (there is a bunch of potentially async stuff going on before the widget is displayed, so the |
|
Spent some more time trying to investigate why the signals don't fire in the tests. A simple function like this works perfectly fine locally, both in Xephyr and Xvfb: -- Same wibox+slider+signals setup as in the test's first step
gears.timer.delayed_call(function()
mouse.coords({ x = 12, y = 24 })
root.fake_input("button_press", 1)
mouse.coords({ x = 100, y = 24 })
root.fake_input("button_release", 1)
end)So clearly, async is not the issue and no waiting between those actions is necessary for the individual signals to fire. The actual reason why no signals fired in CI is that the mouse never actually hit the widget. I don't know why, but the position at which the wibox is placed is not always consistent with what |
Signed-off-by: Lucas Schwiderski <lucas@lschwiderski.de>
| @@ -303,6 +325,14 @@ function slider:set_value(value) | |||
| end | |||
| end | |||
|
|
|||
| --- Returns `true` while the user is dragging the handle. | |||
| -- | |||
| -- @method is_dragging | |||
There was a problem hiding this comment.
sorry for missing this earlier, this should be @property (there is magic to make it work)
There was a problem hiding this comment.
Do you mean "transform this into a property by adding get_is_dragging and set_is_dragging"?
Because there is currently no magic involved. Indexing .is_dragging returns a function, not a boolean.
When trying to hook up the slider to an external value, one would usually have two data streams:
property::value-> set external value.value = new_valueThe problem is that without manual intervention, these two streams form a loop, as setting
.valuealso emitsproperty::value.Having a dedicated "dragging" state with associated signals disconnects these data streams.