mouse handler and collect + store section areas for click locations#129
mouse handler and collect + store section areas for click locations#129otomist wants to merge 5 commits intopythops:masterfrom
Conversation
|
This looks promising!
EDIT: i am stupid. you have correctly identified reusable code for keyboard and mouse user interaction. those changes are relevant and good! |
Thanks for looking at this! I kinda dropped the ball for a bit on this pr. The refactor I needed to do was much larger than I originally expected. I do still want to get it cleaned up and working though. I can try to find time this weekend or next week to look at it again. If the review is too annoying because of the amount of code that changed I can maybe try to split it up into small prs as well. |
sermuns
left a comment
sermuns
left a comment
There was a problem hiding this comment.
I have started reviewing, not done reading all the changes yet!
| adapter_block_bounds: None, | ||
| paired_devices_block_bounds: None, | ||
| new_devices_block_bounds: None, | ||
| help_block_bounds: None, |
There was a problem hiding this comment.
I don't love introducing partial initialization to App.
Do you think we could get with initializing them as Rect::ZERO instead, then resizing once we know?
There was a problem hiding this comment.
Yes looks like a good switch. At first glance I thought it might make it less explicit but after a little thought I think you're right. Using zero for default bounds shouldn't be hard to swap in.
| impl<'a> HelpItem<'a> { | ||
| fn new(spans: Vec<Span<'a>>, action: Option<HelpAction>) -> Self { | ||
| let width: u16 = spans.iter().map(|s| s.content.len() as u16).sum(); | ||
| Self { | ||
| spans, | ||
| x_start: 0, | ||
| x_end: width, | ||
| action, | ||
| } | ||
| } | ||
|
|
||
| fn width(&self) -> u16 { | ||
| self.x_end - self.x_start | ||
| } | ||
|
|
||
| fn set_position(&mut self, x_start: u16) { | ||
| let width = self.width(); | ||
| self.x_start = x_start; | ||
| self.x_end = x_start + width; | ||
| } | ||
|
|
||
| fn get_spans(&self) -> Vec<Span<'a>> { | ||
| self.spans.clone() | ||
| } | ||
|
|
||
| fn to_clickable_item(&self, y: u16) -> ClickableHelpItem { | ||
| ClickableHelpItem { | ||
| x_start: self.x_start, | ||
| x_end: self.x_end, | ||
| y, | ||
| action: self.action, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn place_items_in_row( | ||
| items: &mut [&mut HelpItem<'_>], | ||
| start_x: u16, | ||
| sep_width: u16, | ||
| y: u16, | ||
| clickable_items: &mut Vec<ClickableHelpItem>, | ||
| ) { | ||
| let mut current_x = start_x; | ||
| let last_idx = items.len().saturating_sub(1); | ||
|
|
||
| for (idx, item) in items.iter_mut().enumerate() { | ||
| item.set_position(current_x); | ||
| clickable_items.push(item.to_clickable_item(y)); | ||
|
|
||
| if idx < last_idx { | ||
| current_x = current_x | ||
| .saturating_add(item.width()) | ||
| .saturating_add(sep_width); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems very handrolled. Are you sure this is the only/best way to accomplish this? I'm not saying I know a better way of doing this of the top of my head, but maybe we can explore what Ratatui already offers:
https://ratatui.rs/recipes/layout/grid/
https://ratatui.rs/recipes/layout/dynamic/
https://ratatui.rs/examples/layout/flex/
There was a problem hiding this comment.
I'll take a look at some more options to see if these you linked or other deps in the project can cover this.
2 participants
This PR is to add mouse down event handling so users can click and connect to Bluetooth keyboards when they don't have a wired option available for the keyboard.
Currently in draft mode because I wanted to get approval on the basic logic before making all the changes for different screen sizes.
fixes: #100