Scrollbar LMB click and hold scrolling #3824
Conversation
Frizi
force-pushed
the
wip/frizi/scrollbar-repeat-183390702
branch
from
e6ca13a to
ed24dea
Compare
Frizi
requested review from
MichaelMauderer,
wdanilo,
farmaazon,
mwu-tow and
kazcw
as code owners
| let camera = scene.layers.node_searcher.camera(); | ||
| let display_object = display::object::Instance::new(); | ||
| let masked_layer = layer::Masked::new(&logger, &camera); | ||
| let masked_layer = layer::Masked::new(&logger); |
There was a problem hiding this comment.
Masked layer is probably used by many different parts of the application. Is it? If so, are you 100% sure that removing camera binding here does not introduce any regression?
There was a problem hiding this comment.
I am. This particular change was only affecting scroll-area, and by extension grid-view. I made sure to understand the change and test it manually in all instances. This is essentially a tiny step closer towards cameras being inherited through the layers tree, which as I understand is an eventual goal.
This is in essence a removal of a hard-coded value for one specific view being embedded in a supposedly reusable piece of code.
|
I tried writing a fake browser time support for our mock web bindings. It turns out that this isn't easily doable, because there is an assumption in place that all mocks are ZSTs (there is a transmute that expects that). We would need to preserve the callback in |
| #[derive(Debug, Clone, Copy, Default)] | ||
| pub struct DelayedIntervalConfig { | ||
| /// Initial delay between timer activation and first `on_trigger` event being emitted. | ||
| pub delay_ms: i32, | ||
| /// Time between subsequent `on_trigger` events. | ||
| pub interval_ms: i32, | ||
| } | ||
|
|
||
| impl DelayedIntervalConfig { | ||
| /// Constructor. | ||
| pub fn new(delay_ms: i32, interval_ms: i32) -> Self { | ||
| Self { delay_ms, interval_ms } | ||
| } |
There was a problem hiding this comment.
Weare almost there. This is kind of anti-pattern with FRP. Instead, these config should be set up by FRP inputs set_delay and set_interval. In order to do it, use the define_endpoints_2, store Frp in the DelayedInterval and Deref to it.
There was a problem hiding this comment.
The define_endpoints_2 macro is part of ensogl_core, which depends on the frp crate. I can't use it here.
I am also not thrilled about this DelayedIntervalConfig struct, but I don't really see a great alternative. The main reason it exists it is that you can't change interval values while the timer is running. I wanted an API that makes it very clear that changing the timeout values also restarts the whole timer applying both timeout values at once, and there is no way around that. If I split this into separate set_delay and set_interval streams, the semantics around timer restarts would be potentially surprising.
Avoiding this whole problem was my motivation for using simple boolean based API before. Unfortunately that led to interval values being essentially static, so I changed it to a stream accepting both values at once. Please let me know if you have better ideas how to handle that case.
Frizi
force-pushed
the
wip/frizi/scrollbar-repeat-183390702
branch
from
3e6cd6b to
7bd0a76
Compare
Pull Request Description
Implements https://www.pivotaltracker.com/story/show/183390702
hold-scroll.mp4
Important Notes
This PR adds browser timers (
setTimeoutandsetInterval) support to the FRP core. Those then are used to implement the "hold to scroll repeatedly" functionality.There was a camera-releated bug in the scroll-view debug scene, due to hardcoding made specifically for component browser. This is now fixed, and instead component browser sets the scroll-view camera manually. This is due to change when camera handling in layers will be refactored, and by then no explicit camera setting will be necessary.
There are also some minor extra changes or docs improvements in areas I was going through when looking at FRP implementation.
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide buildand./run ide watch.