feat: scroll with mouse faster#137
Conversation
b236e67 to
d6c67f6
Compare
| } | ||
| LogTabEvent::ScrollDownHalf => { | ||
| self.scroll_log(commander, self.log_height as isize / 2 / 2); | ||
| self.scroll_log(commander, self.log_height as isize / 4); |
There was a problem hiding this comment.
This is of course mathematically correct, but I feel that it looses the essence of what is going on.
log_heightis the number of screen rows available for the list.- First divide by list item height (2), to find the number of items on screen,
- Then divide by two to find half of the number of items that will fit the screen.
I prefer the original, or maybe even adding a let log_height_in_items = log_height / 2
There was a problem hiding this comment.
Well, without your explanation I didn't understand both variants 😄
| ScrollUp, | ||
|
|
||
| ScrollDownRows(isize), | ||
| ScrollUpRows(isize), |
There was a problem hiding this comment.
Why not eliminate ScrollDown / -Up ? ScrollDownRows is more accurate.
There was a problem hiding this comment.
To use it as "default" scroll
| MouseEventKind::ScrollUp if is_big_scroll => { | ||
| Some(LogTabEvent::ScrollUpHalf) | ||
| } | ||
| MouseEventKind::ScrollUp => Some(LogTabEvent::ScrollUp), |
There was a problem hiding this comment.
I think it would be more readable to have a single match on ScrollUp and the if statement inside the arm like this
MouseEventKind::ScrollUp => {
if is_big_scroll { Some(LogTabEvent::ScrollUpHalf) }
else { Some(LogTabEvent::ScrollUp) }
}
There was a problem hiding this comment.
Maybe it's a matter of taste. Also this is formatted as
MouseEventKind::ScrollUp => {
if is_big_scroll {
Some(LogTabEvent::ScrollUpHalf)
} else {
Some(LogTabEvent::ScrollUp)
}
}which is bigger
| DETAILS_PANEL => { | ||
| let action = match mouse_event.kind { | ||
| MouseEventKind::ScrollUp if is_big_scroll => Some(DetailsPanelEvent::ScrollUpHalfPage), | ||
| MouseEventKind::ScrollUp => Some(DetailsPanelEvent::ScrollUpRows(DETAILS_SCROLL)), |
peso
left a comment
peso
left a comment
There was a problem hiding this comment.
It makes more sense to have mouse events for detail panel handled in details_panel.rs
peso
left a comment
peso
left a comment
There was a problem hiding this comment.
I would like the match arms simplified.
| FilesTabEvent::Unbound => return Ok(ComponentInputResult::NotHandled), | ||
| }; | ||
| Ok(ComponentInputResult::Handled) | ||
| } |
There was a problem hiding this comment.
+1 Much easier to understand two smaller functions
|
@istudyatuni is it possible for you to merge this (after resolving conflicts)? |
2 participants
also: