ommp.git

ommp

~ Oh My Music Player — a standalone terminal music player built with Rust and ratatui
git clone https://github.com/debuman/ommp.git
commit 9a4c1e8c642bd141661d2a23fe884a1bd00143be
parent e3b4a7ee271606ab38fba4d9ef678cc16816d62b
Author: sqzer-x <110329244+sqzer-x@users.noreply.github.com>
Date: Sat, 1 Aug 2026 15:44:37 +0900

Make focus follow the click, and make it visible

border_focused and border_unfocused were both Cyan, so every pane computed
a focus-dependent border colour and got the same answer — there was no way to
see which pane had focus at all. They are now distinct.

Focus itself was re-derived from the mouse on every 200ms tick, from a
position latched on the first mouse event and never cleared. Pressing Tab
moved focus for one frame before the next tick dragged it back under the
pointer, and the same clobber fired on any mouse event including the scroll
wheel, which also cancelled the auto-focus after adding to the queue. Focus
now changes on a left click and nothing else; hover still highlights rows.

The hit-testing was duplicated verbatim between the mouse handler and the
tick refresh, so it is one PaneHit now. A click on the library pane's border
or on the blank space below a short list synthesized Enter unconditionally
and activated the previous selection; it is ignored unless the click landed
on a real row, and hover is bounded the same way. Double-click compared only
the row, so a click in one pane paired with a click at the same height in
another; it now requires the same cell and clears once it fires. Mouse events
during the splash were processed against the invisible UI.

Diffstat:
Msrc/app/handler.rs | 221+++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
Msrc/event/input.rs | 5++---
Msrc/main.rs | 17+++++++++++++----
Msrc/ui/mod.rs | 13+++++++++++++
Msrc/ui/panes/library_pane.rs | 6++++++
Msrc/ui/theme.rs | 7+++++--
6 files changed, 172 insertion(+), 97 deletion(-)

diff --git a/src/app/handler.rs b/src/app/handler.rs
@@ -151,13 +151,12 @@ pub fn handle_key_event(key: KeyEvent, app: &App, ui: &mut Ui) -> Vec<AppAction>
                             }
                         }
                     }
-                    KeyCode::Char('r') => {
-                        if !app.playlists.is_empty() {
+                    KeyCode::Char('r')
+                        if !app.playlists.is_empty() => {
                             ui.playlist_modal_mode = PlaylistModalMode::Rename;
                             ui.playlist_modal_input =
                                 app.playlists[ui.playlist_modal_selected].name.clone();
                         }
-                    }
                     _ => {}
                 }
             }
@@ -468,13 +467,12 @@ pub fn handle_mouse_event(
                 if in_results && !ui.search_modal_results.is_empty() {
                     let clicked = ui.search_modal_scroll + (y - ra.y) as usize;
                     if clicked < ui.search_modal_results.len() {
-                        // Double-click detection
-                        let is_double = if let Some((last_time, _lx, ly)) = ui.last_click {
-                            last_time.elapsed() < Duration::from_millis(400) && ly == y
+                        let is_double = is_double_click(ui.last_click, x, y);
+                        ui.last_click = if is_double {
+                            None
                         } else {
-                            false
+                            Some((Instant::now(), x, y))
                         };
-                        ui.last_click = Some((Instant::now(), x, y));
 
                         if is_double {
                             // Double-click: select and confirm (add to queue)
@@ -501,11 +499,10 @@ pub fn handle_mouse_event(
                     }
                 }
             }
-            MouseEventKind::ScrollUp => {
-                if in_results && ui.search_modal_scroll > 0 {
+            MouseEventKind::ScrollUp
+                if in_results && ui.search_modal_scroll > 0 => {
                     ui.search_modal_scroll -= 1;
                 }
-            }
             _ => {}
         }
         return actions;
@@ -517,38 +514,24 @@ pub fn handle_mouse_event(
     }
 
     // Determine which pane the mouse is in
-    let in_library = x >= areas.library.x
-        && x < areas.library.x + areas.library.width
-        && y >= areas.library.y
-        && y < areas.library.y + areas.library.height;
-
-    let in_playlist = x >= areas.playlist.x
-        && x < areas.playlist.x + areas.playlist.width
-        && y >= areas.playlist.y
-        && y < areas.playlist.y + areas.playlist.height;
-
-    let in_lyrics = x >= areas.lyrics.x
-        && x < areas.lyrics.x + areas.lyrics.width
-        && y >= areas.lyrics.y
-        && y < areas.lyrics.y + areas.lyrics.height;
+    let hit = PaneHit::at(&areas, x, y);
+    let in_library = hit.library;
+    let in_playlist = hit.playlist;
+    let in_lyrics = hit.lyrics;
 
     // --- Hover tracking (runs on every mouse event including Moved) ---
     update_hover(ui, &areas, app, x, y, in_library, in_playlist);
+    update_tab_hover(ui, &areas, x, y);
 
-    // --- Tab hover highlight ---
-    if y >= areas.tab_bar.y && y < areas.tab_bar.y + areas.tab_bar.height {
-        ui.hovered_tab = tab_bar::tab_hit_test(areas.tab_bar, x);
-    } else {
-        ui.hovered_tab = None;
-    }
-
-    // --- Focus switching on hover (any mouse event in a pane) ---
-    if in_library && app.focus != FocusedPane::Library {
-        actions.push(AppAction::FocusPane(FocusedPane::Library));
-    } else if in_playlist && app.focus != FocusedPane::Playlist {
-        actions.push(AppAction::FocusPane(FocusedPane::Playlist));
-    } else if in_lyrics && app.focus != FocusedPane::Lyrics {
-        actions.push(AppAction::FocusPane(FocusedPane::Lyrics));
+    // Focus follows the click, not the pointer. Deriving it from hover meant a
+    // scroll — or a tick replaying the last known position — immediately undid
+    // Tab, and undid the auto-focus after adding to the queue.
+    if matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left)) {
+        if let Some(pane) = hit.pane() {
+            if app.focus != pane {
+                actions.push(AppAction::FocusPane(pane));
+            }
+        }
     }
 
     // --- Border drag resize ---
@@ -632,13 +615,17 @@ pub fn handle_mouse_event(
                 }
             }
 
-            // Double-click detection
-            let is_double_click = if let Some((last_time, _last_col, last_row)) = ui.last_click {
-                last_time.elapsed() < Duration::from_millis(400) && last_row == y
+            // Double-click detection. The column has to match too: comparing
+            // only the row made a click in the library followed by a click on
+            // the same screen row of the queue register as a double-click.
+            let is_double_click = is_double_click(ui.last_click, x, y);
+            // Cleared once it fires, so three fast clicks are one double-click
+            // and one single, not two doubles.
+            ui.last_click = if is_double_click {
+                None
             } else {
-                false
+                Some((Instant::now(), x, y))
             };
-            ui.last_click = Some((Instant::now(), x, y));
 
             // Tab bar click
             if y >= areas.tab_bar.y && y < areas.tab_bar.y + areas.tab_bar.height {
@@ -673,8 +660,16 @@ pub fn handle_mouse_event(
                 }
             }
 
-            // Single click in library → select + activate (Enter)
-            if in_library {
+            // Single click in library → select + activate (Enter).
+            // Only when the click actually landed on a row: the synthesized
+            // Enter is unconditional, so a click on the border or the blank
+            // space under a short list used to activate whatever was selected
+            // before — which with a queue-replacing activation meant one stray
+            // click wiped the queue.
+            let clicked_library_row = clicked_row_in(areas.library, y)
+                .map(|row| ui.library_pane_scroll(app) + row)
+                .filter(|&row| row < library_row_count(app, ui));
+            if in_library && clicked_library_row.is_some() {
                 // First, route mouse to pane for selection update
                 let _sel_action = match app.tab {
                     Tab::Queue => ui.library_pane.handle_mouse(mouse, areas.library, app),
@@ -754,6 +749,77 @@ pub fn handle_mouse_event(
     actions
 }
 
+const DOUBLE_CLICK: Duration = Duration::from_millis(400);
+
+/// Row index under `y` inside a bordered pane, or None for the border itself.
+fn clicked_row_in(area: ratatui::layout::Rect, y: u16) -> Option<usize> {
+    let inner = ratatui::widgets::Block::default()
+        .borders(ratatui::widgets::Borders::ALL)
+        .inner(area);
+    (y >= inner.y && y < inner.y + inner.height).then(|| (y - inner.y) as usize)
+}
+
+/// How many rows the library pane is currently showing, so a click below the
+/// last one can be ignored instead of activating whatever was selected before.
+fn library_row_count(app: &App, ui: &Ui) -> usize {
+    match app.tab {
+        Tab::Queue => ui.library_pane.row_count(app),
+        Tab::Directories => ui.dir_browser_pane.entries.len(),
+        Tab::Artists => app.library.get_artists().len(),
+        Tab::Albums => app.library.get_albums().len(),
+        Tab::Genre => app.library.get_genres().len(),
+        Tab::Format => app.library.get_formats().len(),
+        Tab::Playlists => app.playlists.len(),
+    }
+}
+
+/// A second click counts only if it lands on the same cell in time. Matching on
+/// the row alone let a click in one pane pair up with a click in another.
+fn is_double_click(last: Option<(Instant, u16, u16)>, x: u16, y: u16) -> bool {
+    last.is_some_and(|(at, lx, ly)| lx == x && ly == y && at.elapsed() < DOUBLE_CLICK)
+}
+
+/// Which dashboard pane a point lands in. Both the mouse handler and the tick
+/// refresh need this; they used to carry byte-identical copies of the maths.
+struct PaneHit {
+    library: bool,
+    playlist: bool,
+    lyrics: bool,
+}
+
+impl PaneHit {
+    fn at(areas: &LayoutAreas, x: u16, y: u16) -> Self {
+        let inside = |r: ratatui::layout::Rect| {
+            x >= r.x && x < r.x + r.width && y >= r.y && y < r.y + r.height
+        };
+        Self {
+            library: inside(areas.library),
+            playlist: inside(areas.playlist),
+            lyrics: inside(areas.lyrics),
+        }
+    }
+
+    fn pane(&self) -> Option<FocusedPane> {
+        if self.library {
+            Some(FocusedPane::Library)
+        } else if self.playlist {
+            Some(FocusedPane::Playlist)
+        } else if self.lyrics {
+            Some(FocusedPane::Lyrics)
+        } else {
+            None
+        }
+    }
+}
+
+fn update_tab_hover(ui: &mut Ui, areas: &LayoutAreas, x: u16, y: u16) {
+    ui.hovered_tab = if y >= areas.tab_bar.y && y < areas.tab_bar.y + areas.tab_bar.height {
+        tab_bar::tab_hit_test(areas.tab_bar, x)
+    } else {
+        None
+    };
+}
+
 /// Clear all hover_row state across all panes
 fn clear_all_hovers(ui: &mut Ui) {
     ui.queue_pane.hover_row = None;
@@ -794,8 +860,13 @@ fn update_hover(
         let block = ratatui::widgets::Block::default()
             .borders(ratatui::widgets::Borders::ALL);
         let inner = block.inner(areas.library);
+        // Bounded like the queue branch above: hovering the blank space below a
+        // short list used to light up a row that isn't there.
+        let row_count = library_row_count(app, ui);
+        let scroll = ui.library_pane_scroll(app);
         if x >= inner.x && x < inner.x + inner.width
             && y >= inner.y && y < inner.y + inner.height
+            && (scroll + (y - inner.y) as usize) < row_count
         {
             let visual_row = (y - inner.y) as usize;
             match app.tab {
@@ -832,48 +903,23 @@ fn update_hover(
     }
 }
 
-/// Refresh hover state from the stored mouse position.
-/// Call this on Tick events so hover stays updated even without Moved events.
-/// Returns a focus action if the mouse is over a different pane.
-pub fn refresh_hover(app: &App, ui: &mut Ui, terminal_area: ratatui::layout::Rect) -> Vec<AppAction> {
-    let mut actions = Vec::new();
+/// Refresh row highlighting from the stored mouse position, so hover stays
+/// current on terminals that only report motion sporadically.
+///
+/// This deliberately does not touch focus. It used to re-derive focus from
+/// `ui.mouse_pos` on every 200ms tick, and since that position is latched on the
+/// first mouse event and never cleared, pressing Tab moved focus for one frame
+/// before the next tick dragged it back under the pointer.
+pub fn refresh_hover(app: &App, ui: &mut Ui, terminal_area: ratatui::layout::Rect) {
     // Skip hover updates when any modal is open
     if ui.show_about_modal || ui.show_help_modal || ui.show_search_modal || ui.show_playlist_modal {
-        return actions;
-    }
-    if let Some((x, y)) = ui.mouse_pos {
-        let areas = LayoutAreas::compute(terminal_area, ui.pane_widths, ui.right_split);
-        let in_library = x >= areas.library.x
-            && x < areas.library.x + areas.library.width
-            && y >= areas.library.y
-            && y < areas.library.y + areas.library.height;
-        let in_playlist = x >= areas.playlist.x
-            && x < areas.playlist.x + areas.playlist.width
-            && y >= areas.playlist.y
-            && y < areas.playlist.y + areas.playlist.height;
-        let in_lyrics = x >= areas.lyrics.x
-            && x < areas.lyrics.x + areas.lyrics.width
-            && y >= areas.lyrics.y
-            && y < areas.lyrics.y + areas.lyrics.height;
-        update_hover(ui, &areas, app, x, y, in_library, in_playlist);
-
-        // Tab hover highlight
-        if y >= areas.tab_bar.y && y < areas.tab_bar.y + areas.tab_bar.height {
-            ui.hovered_tab = tab_bar::tab_hit_test(areas.tab_bar, x);
-        } else {
-            ui.hovered_tab = None;
-        }
-
-        // Focus switching on hover
-        if in_library && app.focus != FocusedPane::Library {
-            actions.push(AppAction::FocusPane(FocusedPane::Library));
-        } else if in_playlist && app.focus != FocusedPane::Playlist {
-            actions.push(AppAction::FocusPane(FocusedPane::Playlist));
-        } else if in_lyrics && app.focus != FocusedPane::Lyrics {
-            actions.push(AppAction::FocusPane(FocusedPane::Lyrics));
-        }
+        return;
     }
-    actions
+    let Some((x, y)) = ui.mouse_pos else { return };
+    let areas = LayoutAreas::compute(terminal_area, ui.pane_widths, ui.right_split);
+    let hit = PaneHit::at(&areas, x, y);
+    update_hover(ui, &areas, app, x, y, hit.library, hit.playlist);
+    update_tab_hover(ui, &areas, x, y);
 }
 
 /// Resize the focused pane by delta percentage points.
@@ -930,11 +976,10 @@ pub fn update_queue_selection(app: &mut App, key: KeyEvent) {
                 app.queue.selected_index += 1;
             }
         }
-        KeyCode::Char('k') | KeyCode::Up => {
-            if app.queue.selected_index > 0 {
+        KeyCode::Char('k') | KeyCode::Up
+            if app.queue.selected_index > 0 => {
                 app.queue.selected_index -= 1;
             }
-        }
         _ => {}
     }
 }
diff --git a/src/event/input.rs b/src/event/input.rs
@@ -21,11 +21,10 @@ pub fn spawn_input_thread(tx: Sender<Event>) -> std::thread::JoinHandle<()> {
                             break;
                         }
                     }
-                    Ok(CtEvent::Resize(w, h)) => {
-                        if tx.send(Event::Resize(w, h)).is_err() {
+                    Ok(CtEvent::Resize(w, h))
+                        if tx.send(Event::Resize(w, h)).is_err() => {
                             break;
                         }
-                    }
                     _ => {}
                 }
             }
diff --git a/src/main.rs b/src/main.rs
@@ -221,9 +221,17 @@ fn run_app(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>) -> Result<()>
                         }
                     }
                     Event::Mouse(mouse) => {
-                        let size = terminal.size().unwrap_or_default();
-                        let area = ratatui::layout::Rect::new(0, 0, size.width, size.height);
-                        handler::handle_mouse_event(mouse, &app, &mut ui, area)
+                        // Keys are ignored during the splash; mouse events were
+                        // not, so clicks landed on an invisible UI — switching
+                        // tabs, seeking, and replacing the queue.
+                        if ui.show_splash {
+                            vec![]
+                        } else {
+                            let size = terminal.size().unwrap_or_default();
+                            let area =
+                                ratatui::layout::Rect::new(0, 0, size.width, size.height);
+                            handler::handle_mouse_event(mouse, &app, &mut ui, area)
+                        }
                     }
                     Event::Resize(_, _) => {
                         vec![] // Will re-render on next loop
@@ -241,7 +249,8 @@ fn run_app(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>) -> Result<()>
                         // Refresh hover from stored mouse position
                         let size = terminal.size().unwrap_or_default();
                         let area = ratatui::layout::Rect::new(0, 0, size.width, size.height);
-                        handler::refresh_hover(&app, &mut ui, area)
+                        handler::refresh_hover(&app, &mut ui, area);
+                        vec![]
                     }
                     Event::LibraryReady(new_lib) => {
                         app.replace_library(new_lib);
diff --git a/src/ui/mod.rs b/src/ui/mod.rs
@@ -237,6 +237,19 @@ impl Ui {
         }
     }
 
+    /// Scroll offset of whichever pane the library column is currently showing.
+    pub fn library_pane_scroll(&self, app: &App) -> usize {
+        match app.tab {
+            Tab::Queue => self.library_pane.scroll_offset,
+            Tab::Directories => self.dir_browser_pane.scroll_offset,
+            Tab::Artists => self.artists_pane.scroll_offset,
+            Tab::Albums => self.albums_pane.scroll_offset,
+            Tab::Genre => self.genre_pane.scroll_offset,
+            Tab::Format => self.format_pane.scroll_offset,
+            Tab::Playlists => self.playlists_pane.scroll_offset,
+        }
+    }
+
     pub fn refresh_dir_browser(&mut self, app: &App) {
         self.dir_browser_pane.refresh(app);
     }
diff --git a/src/ui/panes/library_pane.rs b/src/ui/panes/library_pane.rs
@@ -39,6 +39,12 @@ impl LibraryPane {
         }
     }
 
+    /// Rows currently on offer, so callers can tell a click on a row from a
+    /// click on the blank space below the list.
+    pub fn row_count(&self, app: &App) -> usize {
+        Self::build_entries(app).len()
+    }
+
     fn build_entries(app: &App) -> Vec<LibraryEntry> {
         let mut entries = Vec::new();
 
diff --git a/src/ui/theme.rs b/src/ui/theme.rs
@@ -25,8 +25,11 @@ impl Default for Theme {
         Self {
             bg: Color::Reset,
             fg: Color::White,
-            border_focused: Color::Cyan,
-            border_unfocused: Color::Cyan,
+            // These were both Cyan, which made the focus indicator a no-op:
+            // every pane picked a border colour from `focused` and got the same
+            // answer, so there was no way to see where focus was.
+            border_focused: Color::Rgb(100, 220, 255),
+            border_unfocused: Color::Rgb(70, 80, 95),
             highlight_bg: Color::Cyan,
             highlight_fg: Color::Black,
             tab_active: Style::default()