Conversation
junkdog
left a comment
junkdog
left a comment
There was a problem hiding this comment.
neat, looking pretty good overall! i had some thoughts on the ScrollDelta/deltaMode handling; comments below.
| let mouse_position = Rc::new(RefCell::new((0, 0))); | ||
| let mouse_button = Rc::new(RefCell::new(None::<MouseButton>)); | ||
| let mouse_event_kind = Rc::new(RefCell::new(None::<MouseEventKind>)); | ||
| let scroll_count = Rc::new(RefCell::new(0i32)); |
There was a problem hiding this comment.
maybe track scrolled and h&v separately, otherwise it can give the impression that ratzilla does not distinguish between the two.
| /// Handles wheel events. | ||
| /// | ||
| /// This method takes a closure that will be called on every `wheel` event. | ||
| fn on_wheel_event<F>(&self, mut callback: F) |
There was a problem hiding this comment.
would it make sense to merge on_wheel_event with on_mouse_event? it's one less closure to care about, so the callback function could handle all mouse event kinds in one place.
| /// - Pages: Multiplied by ~10 notches per page | ||
| pub fn to_steps(self) -> i32 { | ||
| match self { | ||
| ScrollDelta::Pixels(px) => px / 100, |
There was a problem hiding this comment.
px here leads me to believe this is in pixel units, but it's actually converted to rows (or cols) from pixels. (pages is similarly suspect)
where did you get 100 from? i think this value could vary across devices. we probably have to measure it if we care about pixels to rows/cols
There was a problem hiding this comment.
its claude suggested and I approximately tried to verify that and "pages" to approximate 10 wheel events to scroll one page view, etc. But yes, I agree, they are arbitrary, I suspected this would be a problem.
There was a problem hiding this comment.
i tried with:
document.addEventListener('wheel', (e) => {
console.log({
deltaX: e.deltaX,
deltaY: e.deltaY,
deltaMode: ['PIXEL', 'LINE', 'PAGE'][e.deltaMode],
ctrlKey: e.ctrlKey,
shiftKey: e.shiftKey
});
});depending on how quick i scroll the wheel, on at 27" 1440p screen, runing kde/wayland:
- firefox: values range between 16-70px,
- chromium: 16-360(!)
so 100 is definitely too high, esp for ff.
| // Create ScrollDelta based on the browser's delta mode | ||
| let to_scroll_delta = |delta: f64| -> ScrollDelta { | ||
| let delta_int = delta as i32; | ||
| match delta_mode { |
There was a problem hiding this comment.
just a thought, but the more i think about it: would it maybe be easier if we treated all scroll events as either -1 or +1 (or some other increment)? i don't really see pixel-perfect scrolling translating to TUIs. this would omit the need for resolving actual values behind ScrollDelta::Pages and ::Pixels (+ the need to intercept when said values change)
There was a problem hiding this comment.
that's how I use it, but thought it better left to the user to decide if they want to use that information or discard it.
| callback(event.into()); | ||
| }); | ||
| let window = window().unwrap(); | ||
| let document = window.document().unwrap(); |
There was a problem hiding this comment.
you can use get_document() to retrieve it with proper error handling (i know the other on_*_event functions do not, feel free to update them too)
2 participants
No description provided.