Conversation
WalkthroughAdds broad mouse support and navigable UI across multiple screens and widgets, introduces Navigatable caching and mouse APIs, makes UI widgets stateful (nav bar, popups), manages crossterm mouse capture around playback, and includes assorted README, minor MAL, and small rendering/refactor adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Term as crossterm::event
participant Input as InputHandler
participant Manager as ScreenManager
participant Screen as ActiveScreen
participant Nav as Navigatable/Widget
User->>Term: Mouse/Key event
Term->>Input: read()
Input->>Manager: InputEvent::{Key, Mouse}
alt Mouse routed to screen
Manager->>Screen: handle_mouse(event)
Screen->>Nav: get_hovered_index / handle_scroll / handle_mouse
Nav-->>Screen: hovered index / scroll state
alt Click -> action
Screen-->>Manager: Some(Action)
Manager->>Manager: apply Action (switch/show overlay/etc.)
else
Screen-->>Manager: None
end
else Key routed
Manager->>Screen: handle_keyboard(key)
Screen-->>Manager: Optional Action
end
sequenceDiagram
autonumber
participant Main as main()
participant App as App.run()
participant Term as crossterm::event
participant Player as EpisodePlayback
Main->>Term: EnableMouseCapture
Main->>App: run()
App->>Term: DisableMouseCapture (before playback)
App->>Player: start playback
Player-->>App: end playback
App->>Term: EnableMouseCapture (after playback)
note over App,Term: App Drop ensures mouse capture disabled on teardown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/screens/mod.rs (1)
232-256: Don’t swallow navbar selection actions.
In both keyboard and mouse paths we now callself.current_screen.handle_*whenAction::NavbarSelectis returned, but we never bubble the navbar action back up. As a result the app loop never seesAction::NavbarSelectand can’t triggerchange_screen, so selecting a tab while the navbar is focused stops working. Please keep forwarding the original action after any optional screen-level handling.Here’s one way to restore the behavior while still letting screens react:
- if self.navbar.is_selected() { - return self.navbar.handle_keyboard(key_event) - .and_then(|action| match action { - Action::NavbarSelect(_) => self.current_screen.handle_keyboard(key_event), - other => Some(other), - }); - } + if self.navbar.is_selected() { + if let Some(action) = self.navbar.handle_keyboard(key_event) { + if let Action::NavbarSelect(_) = action { + let _ = self.current_screen.handle_keyboard(key_event); + } + return Some(action); + } + return None; + } @@ - if self.navbar.is_selected() { - return self.navbar.handle_mouse(mouse_event) - .and_then(|action| match action { - Action::NavbarSelect(_) => self.current_screen.handle_mouse(mouse_event), - other => Some(other), - }); - } + if self.navbar.is_selected() { + if let Some(action) = self.navbar.handle_mouse(mouse_event) { + if let Action::NavbarSelect(_) = action { + let _ = self.current_screen.handle_mouse(mouse_event); + } + return Some(action); + } + return None; + }src/app.rs (1)
155-166: Don’t unwrap recv() in the main loopA closed input channel will panic the app. Handle RecvError and exit the loop gracefully.
- let first_event = self.rx.recv().unwrap(); + let first_event = match self.rx.recv() { + Ok(e) => e, + Err(_) => { + // sender dropped: shut down cleanly + self.is_running = false; + break; + } + };src/screens/profile.rs (1)
144-151: Prevent u16 underflow when centering favorite anime imageIf computed image_width > image.width, (image.width - image_width) underflows and may wrap in release builds.
Apply:
- let image_width = (image.height as f32 * PICTURE_RATIO * TERMINAL_RATIO) as u16; - let centered_image_area = Rect::new( - image.x + (image.width - image_width) / 2, - image.y, - image_width, - image.height, - ); + let mut image_width = + (image.height as f32 * PICTURE_RATIO * TERMINAL_RATIO) as u16; + image_width = image_width.min(image.width); + let image_x = image.x + (image.width.saturating_sub(image_width)) / 2; + let centered_image_area = + Rect::new(image_x, image.y, image_width, image.height);src/screens/list.rs (1)
177-207: Add missing “by last updated” sort armThe default
sort_byis"by last updated", butsort_animeslacks that case, so lists won’t actually sort by the anime’supdated_at. Add before"by title":match sort_by { + "by last updated" => { + // ISO-8601 strings sort chronologically via lex order + animes.sort_by(|a, b| a.updated_at.cmp(&b.updated_at)); + } "by title" => { animes.sort_by(|a, b| a.title.cmp(&b.title)); }
🧹 Nitpick comments (13)
server/templates/error.html (1)
14-14: Broaden the refresh shortcut wording.Calling out only “ctrl + r” can confuse macOS users who expect “⌘ + R.” Consider phrasing it generically (“refresh the page”) or include both shortcuts so the guidance lands across platforms.
src/screens/widgets/animebox.rs (3)
132-132: Calling.to_string()onformat_dateresult is redundant.The
format_datehelper already returns aString, so the.to_string()call is unnecessary.Apply this diff:
- format_date(&anime.start_date).to_string() + format_date(&anime.start_date)
280-280: Date formatting is inconsistent withAnimeBox.
LongAnimeBoxdisplays raw date strings whileAnimeBox(line 132-138) uses theformat_datehelper for consistent, human-readable date formatting. Consider applying the same formatting here for consistency.Apply this diff to match the formatting used in
AnimeBox:- let airing_text = format!("{} -> {}", anime.start_date, anime.end_date); + let airing_text = if anime.start_date == anime.end_date { + format_date(&anime.start_date) + } else { + format!( + "{}\n->\n{}", + format_date(&anime.start_date), + format_date(&anime.end_date) + ) + };
141-141: Prefer.clone()for consistency withLongAnimeBox.Line 281 uses
.clone()for the same purpose. Sincestatusis already aString, using.clone()is more explicit and consistent throughout the file.Apply this diff:
- let user_stats_value_text = anime.my_list_status.status.to_string(); + let user_stats_value_text = anime.my_list_status.status.clone();server/README.md (1)
10-12: Tidy up the new README section.
Please bump### step 1:to an H2 (##) so the heading hierarchy doesn’t skip a level, and wrap the bare URL in markdown ([MAL API config](https://myanimelist.net/apiconfig)) to silence MD001/MD034 from markdownlint.src/screens/screenTemplate.rs (1)
40-40: Comment tweak LGTMClearer wording. Keep this phrasing consistent across other screens’ caching comments.
src/main.rs (1)
51-53: Mouse capture lifecycle: consider RAII/single ownerEnabling here is fine given App::Drop disables. For tighter ownership and panic safety, consider moving EnableMouseCapture into App::new and keep all capture toggling inside App (matching its Drop).
src/handlers/input_handler.rs (1)
26-32: Coalesce high-frequency MouseEventKind::Moved to avoid event floodForwarding every Moved event can overwhelm the channel and re-render loop. Consider coalescing consecutive Moved events (keep the last) using poll() with a short drain.
Example approach (sketch):
// after reading a Mouse(Moved) event: use crossterm::event::{self, Event as CEvent, MouseEventKind}; use std::time::Duration; let mut last = event; // the first Mouse(Moved) while event::poll(Duration::from_millis(0)) { if let Ok(CEvent::Mouse(m2)) = event::read() { if matches!(m2.kind, MouseEventKind::Moved) { last = CEvent::Mouse(m2); // keep only the latest move continue; } // non-move event encountered; forward it after the loop // (stash it somewhere to be sent next) // break with it saved } else { break; } } // send `last` onceAlso consider whether you need Up() or Drag() for click/drag interactions.
src/app.rs (1)
237-276: Restore TTY state around external playback
Addratatui::restore()before launching the external player to exit raw mode and alt-screen, and re-enter the TUI afterward. This prevents raw-mode artifacts during playback:crossterm::execute!(std::io::stderr(), DisableMouseCapture).ok(); + // restore TTY so external player runs in normal mode + ratatui::restore(); match self.anime_player.play_episode_manually(&anime, next_episode) { ... } - crossterm::execute!(std::io::stderr(), EnableMouseCapture).ok(); - self.terminal = ratatui::init(); + // re-enter TUI after playback + crossterm::execute!(std::io::stderr(), EnableMouseCapture).ok(); + self.terminal = ratatui::init();If the external player doesn’t touch the TTY, leave as-is; otherwise, this avoids garbled output. Ensure the
crosstermfeature is enabled inratatuiand test on your target terminals.src/screens/profile.rs (3)
361-370: Add scroll handling for favorites when hoveredMatch other screens’ behavior by scrolling with the mouse over the favorites grid.
Apply:
+ if self.navigation_fav.is_hovered(mouse_event) { + self.focus = Focus::Content; + self.navigation_fav.handle_scroll(mouse_event); + }
234-236: Round gauge percent to nearest integer and clamp to 100Avoid systematic floor bias; clamp for safety.
Apply:
- let percentage = - (count as f32 / self.user.anime_statistics.num_items as f32 * 100.0) as u16; + let percentage = ((count as f32 * 100.0 + / self.user.anime_statistics.num_items as f32) + .round() as u16) + .min(100);
252-258: Avoid forcing a black gauge backgroundHard-coding .bg(Color::Black) can clash with themes. Let the background inherit.
Apply:
- let gauge = Gauge::default() - .gauge_style( - Style::new() - .fg(anime_list_colors(label)) - .bg(style::Color::Black), - ) + let gauge = Gauge::default() + .gauge_style(Style::new().fg(anime_list_colors(label))) .percent(percentage);src/screens/list.rs (1)
351-370: Avoid per-item linear search in render closureanimes.iter().find(...) is O(n) per tile. Build a small HashMap<AnimeId, &Anime> for O(1) lookups.
Example:
use std::collections::HashMap; let map: HashMap<AnimeId, _> = animes.iter().map(|a| (a.id, a)).collect(); // in closure: if let Some(anime) = map.get(anime_id) { /* render */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
server/README.md(1 hunks)server/templates/error.html(1 hunks)src/app.rs(5 hunks)src/handlers/input_handler.rs(1 hunks)src/main.rs(1 hunks)src/mal/mod.rs(2 hunks)src/mal/network.rs(1 hunks)src/mal/oauth.rs(1 hunks)src/screens/launch.rs(3 hunks)src/screens/list.rs(9 hunks)src/screens/login.rs(5 hunks)src/screens/mod.rs(5 hunks)src/screens/overview.rs(4 hunks)src/screens/profile.rs(13 hunks)src/screens/screenTemplate.rs(1 hunks)src/screens/search.rs(8 hunks)src/screens/seasons.rs(12 hunks)src/screens/widgets/animebox.rs(6 hunks)src/screens/widgets/navbar.rs(7 hunks)src/screens/widgets/navigatable.rs(8 hunks)src/screens/widgets/popup.rs(32 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
src/screens/launch.rs (3)
src/mal/mod.rs (3)
new(33-41)user_is_logged_in(166-179)log_out(150-164)src/screens/login.rs (4)
new(26-37)activate_button(39-62)handle_keyboard(156-170)handle_mouse(172-180)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/login.rs (3)
src/mal/mod.rs (2)
new(33-41)user_is_logged_in(166-179)src/screens/launch.rs (4)
new(22-35)activate_button(37-57)handle_keyboard(115-130)handle_mouse(132-140)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/widgets/navigatable.rs (6)
src/screens/profile.rs (1)
new(61-70)src/screens/launch.rs (1)
new(22-35)src/screens/overview.rs (1)
new(53-80)src/screens/search.rs (1)
new(69-93)src/screens/seasons.rs (1)
new(72-97)src/screens/widgets/popup.rs (5)
new(81-111)new(887-908)new(1288-1302)new(1643-1650)is_hovered(1384-1393)
src/screens/profile.rs (7)
src/config/mod.rs (1)
anime_list_colors(21-30)src/app.rs (2)
new(122-149)update(174-174)src/screens/list.rs (11)
new(40-46)new(59-67)new(119-175)handle_mouse(531-590)update(69-78)update(684-684)update(685-685)update(701-701)update(705-705)update(710-710)animes(616-616)src/screens/overview.rs (7)
new(53-80)handle_mouse(238-258)update(362-362)update(366-366)update(370-370)animes(340-340)animes(352-352)src/screens/widgets/navigatable.rs (1)
new(23-34)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)src/utils/imageManager.rs (1)
query_image_for_fetching(413-439)
src/screens/list.rs (6)
src/screens/overview.rs (4)
animes(340-340)animes(352-352)handle_mouse(238-258)new(53-80)src/screens/search.rs (3)
animes(122-125)handle_mouse(344-389)new(69-93)src/screens/widgets/popup.rs (8)
handle_mouse(495-558)handle_mouse(1024-1098)handle_mouse(1432-1485)handle_mouse(1693-1705)new(81-111)new(887-908)new(1288-1302)new(1643-1650)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)src/app.rs (1)
new(122-149)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/widgets/animebox.rs (2)
src/config/mod.rs (1)
anime_list_colors(21-30)src/utils/stringManipulation.rs (1)
format_date(68-89)
src/screens/seasons.rs (4)
src/screens/list.rs (5)
animes(616-616)new(40-46)new(59-67)new(119-175)handle_mouse(531-590)src/screens/search.rs (3)
animes(122-125)new(69-93)handle_mouse(344-389)src/screens/widgets/popup.rs (8)
new(81-111)new(887-908)new(1288-1302)new(1643-1650)handle_mouse(495-558)handle_mouse(1024-1098)handle_mouse(1432-1485)handle_mouse(1693-1705)src/screens/widgets/navbar.rs (2)
new(26-34)handle_mouse(87-112)
src/screens/search.rs (5)
src/screens/list.rs (4)
new(40-46)new(59-67)new(119-175)handle_mouse(531-590)src/screens/seasons.rs (4)
new(72-97)anime(360-364)anime(366-370)handle_mouse(586-640)src/screens/widgets/navigatable.rs (1)
new(23-34)src/screens/widgets/popup.rs (8)
new(81-111)new(887-908)new(1288-1302)new(1643-1650)handle_mouse(495-558)handle_mouse(1024-1098)handle_mouse(1432-1485)handle_mouse(1693-1705)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)
src/screens/overview.rs (6)
src/screens/profile.rs (4)
handle_mouse(355-373)update(426-426)update(429-429)new(61-70)src/screens/list.rs (11)
handle_mouse(531-590)animes(616-616)update(69-78)update(684-684)update(685-685)update(701-701)update(705-705)update(710-710)new(40-46)new(59-67)new(119-175)src/screens/search.rs (4)
handle_mouse(344-389)animes(122-125)update(435-435)new(69-93)src/screens/seasons.rs (5)
handle_mouse(586-640)animes(138-138)update(682-682)update(685-685)new(72-97)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/widgets/navbar.rs (6)
src/screens/profile.rs (2)
new(61-70)handle_mouse(355-373)src/screens/list.rs (4)
new(40-46)new(59-67)new(119-175)handle_mouse(531-590)src/screens/mod.rs (3)
new(163-180)new(328-333)handle_mouse(123-125)src/screens/overview.rs (2)
new(53-80)handle_mouse(238-258)src/screens/search.rs (2)
new(69-93)handle_mouse(344-389)src/screens/seasons.rs (2)
new(72-97)handle_mouse(586-640)
src/screens/widgets/popup.rs (6)
src/config/mod.rs (2)
anime_list_colors(21-30)new(42-51)src/screens/seasons.rs (4)
anime(360-364)anime(366-370)new(72-97)handle_mouse(586-640)src/mal/models/anime.rs (1)
status_is_known(40-45)src/mal/mod.rs (2)
new(33-41)current_season(181-194)src/screens/search.rs (2)
new(69-93)handle_mouse(344-389)src/screens/widgets/navigatable.rs (2)
new(23-34)is_hovered(86-92)
🪛 markdownlint-cli2 (0.18.1)
server/README.md
10-10: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
11-11: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (37)
src/screens/widgets/animebox.rs (4)
51-54: LGTM! Cleaner color selection logic.The simplified else-if structure correctly handles the three cases: highlighted, empty status, and status-based coloring.
112-112: LGTM! Cleaner argument passing.Passing
image_managerdirectly (instead of&image_manager) is more idiomatic since the parameter is already a reference type&Arc<Mutex<ImageManager>>.
267-267: LGTM! Cleaner argument passing.Passing
image_managerdirectly (instead of&image_manager) is more idiomatic since the parameter is already a reference type&Arc<Mutex<ImageManager>>.
281-281: LGTM! Using.clone()is appropriate here.Since
Paragraph::new()takes ownership of the string, cloning the status field is the correct approach.src/mal/oauth.rs (1)
56-57: No issues spotted.
open::thatremains correctly referenced via the crate path; the cleanup doesn’t affect behavior.src/mal/mod.rs (2)
187-190: Inclusive month ranges look good.
Switching to1..=3/4..=6/7..=9matches chrono’s 1-based months and keeps fall as the catch-all.
393-394: Consistent error reporting.
Forwarding fetch errors throughsend_error!aligns this call site with the rest of the module.src/mal/network.rs (1)
67-68: Style tweak is fine.
Returning the error arm directly keeps semantics identical and reads cleaner.src/app.rs (1)
332-335: Teardown sequence LGTMRestoring terminal and disabling mouse in Drop is correct and idempotent.
If you adopt the pre-playback ratatui::restore, ensure there’s no double-restore path on early panics (manual test is enough).
src/screens/overview.rs (2)
238-256: Mouse handling reads well and matches other screensHover-to-focus, per-list scroll, and click-to-overlay are consistent with list/search screens.
Confirm that Navigatable::get_hovered_item_mut correctly resolves per-section areas after the extra top margin applied in draw().
288-289: Direct AnimeId collection LGTMCollecting ids via
.map(|a| a.id)avoids clones and matches apply_update() expectations.Also applies to: 340-345, 352-357
src/screens/search.rs (3)
18-19: Search box hit-testing state looks goodImporting Position, storing search_area, and assigning it post-render enables reliable mouse focus for the search field.
Also applies to: 61-62, 85-88, 223-224
344-365: Filter popup mouse flow is consistent and prioritizedHover/open detection + early return on non-selection matches the UX in other screens; normalization to "bypopularity" before dispatch is correct.
324-328: Enter-to-open overlay via store resolutionUsing app_info.anime_store.get(anime_id) to resolve and ShowOverlay(anime.id) is correct and avoids stale IDs.
src/screens/list.rs (3)
351-370: Use pre-fetched visible items with get_bulk — good changeFetching only visible animes reduces work and flicker; closure highlights only when in Content focus.
531-590: Mouse handling mirrors Search screen — consistent UXTop-row NavBar handoff, dropdown priority, search-area focus, list hover/scroll, and click-to-overlay all look correct.
613-621: Efficient ID extraction in background updatesMapping animes.iter().map(|a| a.id) and emitting only IDs for filtered/search results keeps UI responsive.
Also applies to: 670-675
src/screens/login.rs (5)
17-37: LGTM! Clean refactoring to use Navigatable.The struct changes align well with the broader mouse support initiative, replacing manual button tracking with the Navigatable pattern used elsewhere in the codebase.
156-170: LGTM! Keyboard handling correctly delegates to Navigatable.The pattern matches the implementation in launch.rs and properly separates navigation from action triggering.
172-180: LGTM! Mouse handling follows established pattern.The implementation correctly checks for click events and delegates to
activate_button.
203-215: LGTM! Smart separation of full URL from display URL.Sending the
full_urlimmediately enables the "Open Browser" button before the typing animation completes, improving responsiveness.
226-233: LGTM! Update handler correctly processes both URL fields.src/screens/widgets/navigatable.rs (7)
16-17: LGTM! Cache fields added for performance.The
areaandgridcaching will avoid redundant layout calculations when rendering with the same area repeatedly.
35-60: LGTM! Hovered index calculation correctly handles scrolling and bounds.The method properly:
- Checks area containment
- Calculates absolute index from visible position and scroll offset
- Validates bounds against total_items
- Updates selection state
62-70: LGTM! Helper methods provide convenient item access.
86-92: LGTM! Boundary check for mouse hover detection.
94-110: LGTM! Scroll handling correctly adjusts by row increments with bounds checks.The method properly:
- Scrolls by column count (one row)
- Prevents underflow with
saturating_sub- Prevents scrolling past the last visible elements
196-220: LGTM! Grid caching avoids redundant layout calculations.The cache check at the start returns early when the area hasn't changed. The actual cache update happens in
constructandconstruct_mut.
264-267: LGTM! Selection setter ensures scroll stays in sync.src/screens/widgets/popup.rs (8)
74-78: LGTM! Cache fields enable mouse interaction.
315-355: LGTM! Status update logic correctly prevents invalid state.The method properly handles the edge case where watching episodes without a status should default to "watching", while preventing list additions when nothing is selected.
495-558: LGTM! Mouse handler correctly prioritizes interactions and delegates to keyboard logic.The method properly:
- Prioritizes open dropdowns over hover
- Handles synopsis scrolling
- Closes on outside clicks
- Reuses keyboard logic for button activation
581-581: LGTM! Popup area cached for mouse interaction.
932-943: LGTM! Toggle API simplified by tracking previous selection.The
toggle()method no longer requires ayearparameter becauseprevious_yearmaintains state across toggles. This is cleaner and aligns with the new mouse interaction model.
1024-1098: LGTM! Mouse handler correctly manages season selection and closing behavior.The method properly:
- Toggles on activate area clicks
- Handles scrolling in year/season regions
- Selects and closes on inside clicks
- Closes on outside clicks
1432-1485: LGTM! SelectionPopup mouse handler correctly manages scrolling and selection.The method properly:
- Toggles on button clicks
- Handles scroll events within popup
- Maps clicked rows to options accounting for scroll offset
- Closes on outside clicks
1535-1540: LGTM! Auto-scroll keeps selected option visible.The scroll adjustment ensures the highlighted option remains in the visible range when navigating with keyboard or mouse.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/screens/overview.rs (1)
296-296: map_while truncates valid entries after the first error.Using
map_whilestops iteration at the first line read error, discarding all subsequent valid log entries. For a watch history log file, you likely want to skip corrupted lines and continue processing.Apply this diff to restore the original behavior:
- let entries: Vec<String> = content.lines().map_while(Result::ok).collect(); + let entries: Vec<String> = content.lines().filter_map(Result::ok).collect();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/login.rs(5 hunks)src/screens/overview.rs(4 hunks)src/screens/profile.rs(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/screens/overview.rs (5)
src/screens/profile.rs (5)
handle_mouse(354-372)update(425-425)update(428-428)update(431-431)new(61-70)src/screens/search.rs (3)
handle_mouse(344-389)animes(122-125)new(69-93)src/screens/list.rs (11)
handle_mouse(531-590)animes(616-616)update(69-78)update(684-684)update(685-685)update(701-701)update(705-705)update(710-710)new(40-46)new(59-67)new(119-175)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/login.rs (4)
src/mal/mod.rs (2)
new(33-41)user_is_logged_in(166-179)src/screens/launch.rs (4)
new(22-35)activate_button(37-57)handle_keyboard(115-130)handle_mouse(132-140)src/screens/mod.rs (4)
new(163-180)new(328-333)handle_keyboard(120-122)handle_mouse(123-125)src/screens/widgets/navigatable.rs (1)
new(23-34)
src/screens/profile.rs (6)
src/config/mod.rs (1)
anime_list_colors(21-30)src/screens/overview.rs (4)
new(53-80)handle_mouse(238-258)animes(340-340)animes(352-352)src/screens/search.rs (3)
new(69-93)handle_mouse(344-389)animes(122-125)src/screens/widgets/navigatable.rs (1)
new(23-34)src/screens/widgets/navbar.rs (1)
handle_mouse(87-112)src/utils/imageManager.rs (1)
query_image_for_fetching(413-439)
🔇 Additional comments (23)
src/screens/login.rs (9)
4-4: LGTM!The import of
Navigatablealigns with the navigation refactor across other screens.
18-22: LGTM!The struct fields properly support the new Navigatable-based UI and store the full OAuth URL separately from the typed display URL.
26-37: LGTM!The constructor properly initializes all fields. The
Navigatable::new((2, 1))correctly matches the two-button vertical layout.
39-62: Error handling correctly implemented.The error handling for
open::that()at lines 46-48 properly addresses the previous review comment - errors are no longer silently discarded and are returned asAction::ShowError. The method signature change to&mut selfis necessary for clearinglogin_urlon line 54.
133-153: LGTM!The rendering logic properly delegates to
Navigatablefor button layout and selection state management, consistent with the pattern inlaunch.rs.
156-170: LGTM!Keyboard handling correctly delegates navigation to
Navigatableand activates buttons on Enter, consistent withlaunch.rs.
172-180: LGTM!Mouse handling correctly activates buttons on click via
Navigatable::get_hovered_index(), consistent with the pattern inlaunch.rs.
203-214: LGTM!The background thread properly sends
full_urlimmediately for button functionality while animatinglogin_urlfor visual effect. This separation is a good design choice.
230-232: LGTM!The
apply_updatemethod correctly handlesfull_urlupdates from the background thread, mirroring the existinglogin_urlhandling.src/screens/overview.rs (3)
238-258: LGTM! Mouse handling correctly implements hierarchical navigation.The implementation properly handles the two-level navigation structure (outer list selection, then inner anime selection within that list). The pattern of using
get_hovered_item_mutto identify which list is hovered, thenget_hovered_itemto identify the specific anime, is appropriate for this screen's layout.
288-288: LGTM! Direct ID access is cleaner.The change to use
.iddirectly instead of cloning is appropriate and more readable.
340-340: LGTM! Consistent ID extraction pattern.Both changes correctly extract anime IDs for background updates, maintaining consistency with the WatchHistory pattern at line 325.
Also applies to: 352-352
src/screens/profile.rs (11)
10-10: LGTM! Imports align with usage.The
anime_list_colorsandGaugeimports support the gauge rendering logic for anime statistics introduced in this PR.Also applies to: 39-39
63-63: LGTM! Initial focus set to NavBar.Setting the initial focus to
NavBaris consistent with the navigation pattern used in other screens and supports the new mouse navigation flow.
179-186: LGTM! Array simplifies user info structure.Changing
user_info_textfrom aVecto an array is appropriate for this fixed-size collection and slightly improves clarity.
199-204: LGTM! Y-coordinate calculation fixed.The area calculation now correctly uses
right_top.y + right_top.height / 2for absolute positioning, which resolves the issue flagged in the previous review.
257-274: LGTM! Gauge rendering improved with status-based coloring.The gauge styling now uses
anime_list_colors(label)for the foreground, providing visual differentiation by anime status. The margin adjustments and label positioning are correct.
277-304: LGTM! Statistics rendering consolidated.The multi-line string formatting for totals is cleaner and maintains correct alignment.
318-322: LGTM! Navigation shortcut improved.Adding
KeyCode::Upas an alternative toCtrl+Jimproves keyboard navigation consistency with other screens.
337-344: LGTM! Enter key now triggers overlay.The Enter key handling correctly retrieves the selected favorite anime and shows the overlay, consistent with the navigation pattern in other screens.
355-358: LGTM! Navbar mouse handling consistent with other screens.The navbar area detection and focus switching match the pattern used in
overview.rsandsearch.rs.
416-418: LGTM! Favorited animes update construction simplified.Inlining the update construction is cleaner and maintains the same functionality.
405-408: Ignore removal suggestion foranimes; it's used insrc/app.rs.
Theanimesfield is consumed insrc/app.rs:174viaupdate.take::<Vec<Anime>>("animes"), so it must be retained.Likely an incorrect or invalid review comment.
| if self.navigation_fav.get_hovered_index(mouse_event).is_some() { | ||
| self.focus = Focus::Content; | ||
|
|
||
| if let crossterm::event::MouseEventKind::Down(_) = mouse_event.kind { | ||
| let anime_id = self | ||
| .navigation_fav | ||
| .get_selected_item(&self.user.favorited_animes)?; | ||
| return Some(Action::ShowOverlay(anime_id.id)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Consider adding scroll handling for favorited animes.
The click handling correctly retrieves the selected favorite anime and triggers the overlay. However, unlike overview.rs:252 and search.rs:378, this method doesn't call handle_scroll(mouse_event) to support mouse wheel scrolling over the favorited animes grid.
Verify whether scroll handling should be added:
If Navigatable::handle_scroll exists and is used in other screens for mouse wheel support, consider adding it here:
if self.navigation_fav.get_hovered_index(mouse_event).is_some() {
self.focus = Focus::Content;
+ self.navigation_fav.handle_scroll(mouse_event);
if let crossterm::event::MouseEventKind::Down(_) = mouse_event.kind {🏁 Script executed:
#!/bin/bash
# Description: Check if Navigatable supports handle_scroll and if it's used elsewhere
# Search for handle_scroll implementations in Navigatable
ast-grep --pattern 'impl Navigatable {
$$$
fn handle_scroll($$$) {
$$$
}
$$$
}'
# Search for handle_scroll usage in other screen files
rg -nP --type=rust 'navigation.*\.handle_scroll\(' src/screens/Length of output: 0
🏁 Script executed:
#!/bin/bash
rg -n 'trait Navigatable' -g '*.rs'
rg -n 'impl Navigatable' -g '*.rs'
rg -n 'handle_scroll' -g '*.rs'Length of output: 476
🏁 Script executed:
#!/bin/bash
sed -n '330,400p' src/screens/profile.rsLength of output: 2632
Add scroll handling for favorited animes
Insert self.navigation_fav.handle_scroll(mouse_event); after setting focus in handle_mouse to support mouse-wheel scrolling, mirroring other screens:
if self.navigation_fav.get_hovered_index(mouse_event).is_some() {
self.focus = Focus::Content;
+ self.navigation_fav.handle_scroll(mouse_event);
if let crossterm::event::MouseEventKind::Down(_) = mouse_event.kind {
let anime_id = self
.navigation_fav
.get_selected_item(&self.user.favorited_animes)?;
return Some(Action::ShowOverlay(anime_id.id));
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if self.navigation_fav.get_hovered_index(mouse_event).is_some() { | |
| self.focus = Focus::Content; | |
| if let crossterm::event::MouseEventKind::Down(_) = mouse_event.kind { | |
| let anime_id = self | |
| .navigation_fav | |
| .get_selected_item(&self.user.favorited_animes)?; | |
| return Some(Action::ShowOverlay(anime_id.id)); | |
| } | |
| } | |
| if self.navigation_fav.get_hovered_index(mouse_event).is_some() { | |
| self.focus = Focus::Content; | |
| self.navigation_fav.handle_scroll(mouse_event); | |
| if let crossterm::event::MouseEventKind::Down(_) = mouse_event.kind { | |
| let anime_id = self | |
| .navigation_fav | |
| .get_selected_item(&self.user.favorited_animes)?; | |
| return Some(Action::ShowOverlay(anime_id.id)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/screens/profile.rs around lines 360 to 369, after you set self.focus =
Focus::Content when the favorite navigation is hovered, call
self.navigation_fav.handle_scroll(mouse_event); to enable mouse-wheel scrolling
for favorited animes (place this immediately after the focus assignment and
before the existing match on mouse_event.kind). Ensure the call uses the same
mouse_event variable and retains the existing early return behavior when a Down
event selects an item.
1 participant
Added mouse support:
Summary by CodeRabbit
New Features
Improvements
Documentation