From 94d63c9bc396ed14e4e1658395e968cd443c8351 Mon Sep 17 00:00:00 2001 From: oskarrough Date: Sat, 25 Oct 2025 22:22:58 +0200 Subject: [PATCH] refactor phase 1: extract view modules with Action pattern Change-Id: sxnxmyvklnlklwrluwsqxkmnoyvkoqnx - extracted channels_view.rs module (State, Message, Action) - extracted tracks_view.rs module (State, Message, Action) - moved filtering/sorting logic into view modules - updated main.rs to route messages and handle Actions - follows iced composable architecture pattern - prepares for phase 2 (PaneContent enum) and phase 3 (DataStore) See CHANGELOG.md for detailed narrative. --- CHANGELOG.md | 21 +++ src/channels_view.rs | 193 +++++++++++++++++++++++++ src/main.rs | 331 ++++++------------------------------------- src/tracks_view.rs | 200 ++++++++++++++++++++++++++ todo.txt | 46 ++++++ 5 files changed, 500 insertions(+), 291 deletions(-) create mode 100644 src/channels_view.rs create mode 100644 src/tracks_view.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 87eea8f..6063097 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ this is a narrative thread, not a commit log. we tell the story of what we're building. +--- + +2025-10-25 (continued) +---------------------- + +refactored architecture (phase 1/3): extracted view modules with Action pattern + +following iced docs "Scaling Applications" pattern, separated view concerns from main state. created channels_view.rs and tracks_view.rs modules, each with their own State, Message enum, and Action enum for parent communication. + +improvements: + - #refactor extracted channels_view module (State, Message, Action with ShowChannelTracks) + - #refactor extracted tracks_view module (State, Message, Action, multi-select logic) + - #refactor moved filtering/sorting logic into view modules (filtered_channels, sorted_channels, filtered_tracks, sorted_tracks) + - #refactor updated main update() to route view messages and handle Actions + - #refactor view state stored in main State (channels_view_state, tracks_view_state) + - #pattern follows iced composable architecture (update returns Action, parent handles side effects) + - #cleanup removed duplicate code from main.rs (render functions simplified, matches_query removed) + - #code reduced main.rs complexity (view logic encapsulated, cleaner message routing) + +benefits: clear separation of concerns, testable view logic, type-safe transitions via Actions. prepares for phase 2 (PaneContent enum) and phase 3 (DataStore separation). + --- 2025-10-25 diff --git a/src/channels_view.rs b/src/channels_view.rs new file mode 100644 index 0000000..a4180ba --- /dev/null +++ b/src/channels_view.rs @@ -0,0 +1,193 @@ +use crate::channel::Channel; +use crate::palette::Palette; +use crate::view; +use iced::widget::{column, container, horizontal_rule, scrollable, text}; +use iced::{Element, Length, Theme}; + +#[derive(Debug, Clone)] +pub struct State { + pub selected_channel: Option, +} + +impl Default for State { + fn default() -> Self { + Self { + selected_channel: None, + } + } +} + +#[derive(Debug, Clone)] +pub enum Message { + ChannelSelected(usize), +} + +#[derive(Debug, Clone)] +pub enum Action { + None, + ShowChannelTracks(String, String), +} + +impl State { + pub fn new() -> Self { + Self::default() + } + + pub fn update(&mut self, message: Message, channels: &[Channel]) -> Action { + match message { + Message::ChannelSelected(idx) => { + self.selected_channel = Some(idx); + if let Some(channel) = channels.get(idx) { + Action::ShowChannelTracks(channel.slug.clone(), channel.name.clone()) + } else { + Action::None + } + } + } + } + + pub fn view<'a>( + &'a self, + all_channels: &'a [Channel], + view_def: &'a view::View, + all_tracks: &'a [crate::track::Track], + palette: &'a Palette, + ) -> Element<'a, Message> { + let channels = sorted_channels( + filtered_channels(all_channels, view_def), + view_def, + all_tracks, + ); + let list = channels + .iter() + .fold(column![].spacing(0), |col, (idx, channel)| { + let is_selected = self.selected_channel == Some(*idx); + let channel_info = format!("{} ({})", channel.name, channel.track_count); + + let item = container(text(channel_info).size(14)) + .padding(8) + .width(Length::Fill) + .style(move |_theme: &Theme| { + if is_selected { + container::Style { + background: Some(iced::Background::Color(palette.gray3)), + ..Default::default() + } + } else { + container::Style::default() + } + }); + + let button = iced::widget::button(item) + .on_press(Message::ChannelSelected(*idx)) + .padding(0) + .style(|theme: &Theme, _status| iced::widget::button::Style { + background: None, + text_color: theme.palette().text, + border: iced::Border::default(), + shadow: iced::Shadow::default(), + }); + + let col = col.push(button); + col.push( + horizontal_rule(1).style(move |_theme: &Theme| iced::widget::rule::Style { + color: palette.gray3, + width: 1, + radius: 0.0.into(), + fill_mode: iced::widget::rule::FillMode::Full, + }), + ) + }); + + let content = scrollable(list); + + container(content) + .width(Length::Fill) + .height(Length::Fill) + .style(move |_theme: &Theme| container::Style { + background: Some(iced::Background::Color(palette.gray4)), + ..Default::default() + }) + .into() + } +} + +pub fn filtered_channels<'a>( + channels: &'a [Channel], + view: &view::View, +) -> Vec<(usize, &'a Channel)> { + channels + .iter() + .enumerate() + .filter(|(_idx, channel)| { + if let Some(ref slug) = view.filter.slug { + if &channel.slug != slug { + return false; + } + } + + if let Some(min) = view.filter.track_count_min { + if channel.track_count < min { + return false; + } + } + + if let Some(max) = view.filter.track_count_max { + if channel.track_count > max { + return false; + } + } + + if !view.filter.search_query.is_empty() + && !matches_query(&channel.name, &view.filter.search_query) + && !matches_query(&channel.slug, &view.filter.search_query) + { + return false; + } + + true + }) + .collect() +} + +pub fn sorted_channels<'a>( + channels: Vec<(usize, &'a Channel)>, + view: &view::View, + all_tracks: &[crate::track::Track], +) -> Vec<(usize, &'a Channel)> { + let mut sorted = channels; + + match view.sort.field { + view::SortField::Name => { + sorted.sort_by(|(_, a), (_, b)| a.name.cmp(&b.name)); + } + view::SortField::TrackCount => { + sorted.sort_by_key(|(_, ch)| ch.track_count); + } + view::SortField::LatestTrack => { + sorted.sort_by_key(|(_, channel)| { + all_tracks + .iter() + .filter(|t| t.slug == channel.slug) + .map(|t| &t.created_at) + .max() + .cloned() + .unwrap_or_default() + }); + } + view::SortField::CreatedAt => {} + } + + if matches!(view.sort.direction, view::SortDirection::Desc) { + sorted.reverse(); + } + + sorted +} + +fn matches_query(text: &str, query: &str) -> bool { + if query.is_empty() { + return true; + } + text.to_lowercase().contains(&query.to_lowercase()) +} diff --git a/src/main.rs b/src/main.rs index 4f41169..b029d08 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,14 +1,14 @@ mod channel; +mod channels_view; mod palette; mod panel; mod track; +mod tracks_view; mod view; mod view_editor; use iced::font::Font; -use iced::widget::{ - button, column, container, horizontal_rule, pane_grid, row, scrollable, text, text_input, -}; +use iced::widget::{button, column, container, pane_grid, row, text, text_input}; use iced::{Length, Padding, Theme}; use channel::Channel; use palette::Palette; @@ -62,8 +62,8 @@ struct State { panes: pane_grid::State, channels: Vec, tracks: Vec, - selected_channel: Option, - selected_tracks: Vec, + channels_view_state: channels_view::State, + tracks_view_state: tracks_view::State, modifiers: iced::keyboard::Modifiers, palette: Palette, views: Vec, @@ -103,8 +103,8 @@ impl Default for State { panes, channels, tracks, - selected_channel: None, - selected_tracks: Vec::new(), + channels_view_state: channels_view::State::new(), + tracks_view_state: tracks_view::State::new(), modifiers: iced::keyboard::Modifiers::default(), palette: Palette::default(), views, @@ -115,8 +115,8 @@ impl Default for State { #[derive(Debug, Clone)] enum Message { - ChannelSelected(usize), - TrackClicked(usize), + ChannelsView(channels_view::Message), + TracksView(tracks_view::Message), ModifiersChanged(iced::keyboard::Modifiers), Pane(PaneMessage), SearchChanged(pane_grid::Pane, String), @@ -127,60 +127,36 @@ enum Message { ViewEditor(view_editor::Message), } -fn matches_query(text: &str, query: &str) -> bool { - if query.is_empty() { - return true; - } - text.to_lowercase().contains(&query.to_lowercase()) -} - fn update(state: &mut State, message: Message) { match message { - Message::ChannelSelected(idx) => { - state.selected_channel = Some(idx); - - if let Some(channel) = state.channels.get(idx) { - let view = view::View::channel_tracks(&channel.slug, &channel.name); - let view_id = view.id.clone(); - - if !state.views.iter().any(|v| v.id == view_id) { - state.views.push(view); - } + Message::ChannelsView(msg) => { + let action = state.channels_view_state.update(msg, &state.channels); + match action { + channels_view::Action::ShowChannelTracks(slug, name) => { + let view = view::View::channel_tracks(&slug, &name); + let view_id = view.id.clone(); + + if !state.views.iter().any(|v| v.id == view_id) { + state.views.push(view); + } - if let Some((_pane_id, content)) = state.panes.iter_mut().find(|(_, c)| { - c.current_view_id() - .map(|id| id != "all-channels") - .unwrap_or(false) - }) { - content.push_view(view_id); + if let Some((_pane_id, content)) = state.panes.iter_mut().find(|(_, c)| { + c.current_view_id() + .map(|id| id != "all-channels") + .unwrap_or(false) + }) { + content.push_view(view_id); + } } + channels_view::Action::None => {} } } - Message::TrackClicked(index) => { - let ctrl = state.modifiers.command() || state.modifiers.control(); - let shift = state.modifiers.shift(); - - if ctrl { - if state.selected_tracks.contains(&index) { - state.selected_tracks.retain(|&i| i != index); - } else { - state.selected_tracks.push(index); - } - } else if shift { - if let Some(&last) = state.selected_tracks.last() { - let start = last.min(index); - let end = last.max(index); - state.selected_tracks.clear(); - state.selected_tracks.extend(start..=end); - } else { - state.selected_tracks = vec![index]; - } - } else { - state.selected_tracks = vec![index]; - } + Message::TracksView(msg) => { + let _action = state.tracks_view_state.update(msg); } Message::ModifiersChanged(modifiers) => { state.modifiers = modifiers; + state.tracks_view_state.modifiers = modifiers; } Message::SearchChanged(pane, query) => { if let Some(content) = state.panes.get(pane) { @@ -425,8 +401,8 @@ fn render_pane_content<'a>( }; let result_count = match view.source { - view::DataSource::Channels => filtered_channels(state, view).len(), - view::DataSource::Tracks => filtered_tracks(state, view).len(), + view::DataSource::Channels => channels_view::filtered_channels(&state.channels, view).len(), + view::DataSource::Tracks => tracks_view::filtered_tracks(&state.tracks, view).len(), }; let search_placeholder = match view.source { @@ -458,8 +434,14 @@ fn render_pane_content<'a>( }); let view_content = match view.source { - view::DataSource::Channels => render_channels_view(view, state, palette), - view::DataSource::Tracks => render_tracks_view(view, state, palette), + view::DataSource::Channels => state + .channels_view_state + .view(&state.channels, view, &state.tracks, palette) + .map(Message::ChannelsView), + view::DataSource::Tracks => state + .tracks_view_state + .view(&state.tracks, view, palette) + .map(Message::TracksView), }; column![tabs, search_bar, view_content] @@ -468,236 +450,3 @@ fn render_pane_content<'a>( .into() } -fn filtered_channels<'a>(state: &'a State, view: &view::View) -> Vec<(usize, &'a Channel)> { - state - .channels - .iter() - .enumerate() - .filter(|(_idx, channel)| { - if let Some(ref slug) = view.filter.slug { - if &channel.slug != slug { - return false; - } - } - - if let Some(min) = view.filter.track_count_min { - if channel.track_count < min { - return false; - } - } - - if let Some(max) = view.filter.track_count_max { - if channel.track_count > max { - return false; - } - } - - if !view.filter.search_query.is_empty() - && !matches_query(&channel.name, &view.filter.search_query) - && !matches_query(&channel.slug, &view.filter.search_query) - { - return false; - } - - true - }) - .collect() -} - -fn render_channels_view<'a>( - view: &'a view::View, - state: &'a State, - palette: &'a Palette, -) -> iced::Element<'a, Message> { - let mut sorted_channels = filtered_channels(state, view); - - match view.sort.field { - view::SortField::Name => { - sorted_channels.sort_by(|(_, a), (_, b)| a.name.cmp(&b.name)); - } - view::SortField::TrackCount => { - sorted_channels.sort_by_key(|(_, ch)| ch.track_count); - } - view::SortField::LatestTrack => { - sorted_channels.sort_by_key(|(_, channel)| { - state - .tracks - .iter() - .filter(|t| t.slug == channel.slug) - .map(|t| &t.created_at) - .max() - .cloned() - .unwrap_or_default() - }); - } - view::SortField::CreatedAt => {} - } - - if matches!(view.sort.direction, view::SortDirection::Desc) { - sorted_channels.reverse(); - } - - let list = sorted_channels - .iter() - .fold(column![].spacing(0), |col, (idx, channel)| { - let is_selected = state.selected_channel == Some(*idx); - let channel_info = format!("{} ({})", channel.name, channel.track_count); - - let item = container(text(channel_info).size(14)) - .padding(8) - .width(Length::Fill) - .style(move |_theme: &Theme| { - if is_selected { - container::Style { - background: Some(iced::Background::Color(palette.gray3)), - ..Default::default() - } - } else { - container::Style::default() - } - }); - - let button = iced::widget::button(item) - .on_press(Message::ChannelSelected(*idx)) - .padding(0) - .style(|theme: &Theme, _status| iced::widget::button::Style { - background: None, - text_color: theme.palette().text, - border: iced::Border::default(), - shadow: iced::Shadow::default(), - }); - - let col = col.push(button); - col.push( - horizontal_rule(1).style(move |_theme: &Theme| iced::widget::rule::Style { - color: palette.gray3, - width: 1, - radius: 0.0.into(), - fill_mode: iced::widget::rule::FillMode::Full, - }), - ) - }); - - let content = scrollable(list); - - container(content) - .width(Length::Fill) - .height(Length::Fill) - .style(move |_theme: &Theme| container::Style { - background: Some(iced::Background::Color(palette.gray4)), - ..Default::default() - }) - .into() -} - -fn filtered_tracks<'a>(state: &'a State, view: &view::View) -> Vec<(usize, &'a Track)> { - state - .tracks - .iter() - .enumerate() - .filter(|(_idx, track)| { - if let Some(ref slug) = view.filter.slug { - if &track.slug != slug { - return false; - } - } - - if !view.filter.search_query.is_empty() - && !matches_query(&track.title, &view.filter.search_query) - && !matches_query(&track.description, &view.filter.search_query) - && !matches_query(&track.slug, &view.filter.search_query) - { - return false; - } - - true - }) - .collect() -} - -fn render_tracks_view<'a>( - view: &'a view::View, - state: &'a State, - palette: &'a Palette, -) -> iced::Element<'a, Message> { - let mut sorted_tracks = filtered_tracks(state, view); - - match view.sort.field { - view::SortField::Name => { - sorted_tracks.sort_by(|(_, a), (_, b)| a.title.cmp(&b.title)); - } - view::SortField::CreatedAt => { - sorted_tracks.sort_by(|(_, a), (_, b)| a.created_at.cmp(&b.created_at)); - } - view::SortField::TrackCount | view::SortField::LatestTrack => {} - } - - if matches!(view.sort.direction, view::SortDirection::Desc) { - sorted_tracks.reverse(); - } - - sorted_tracks.truncate(5000); - - let list = sorted_tracks - .iter() - .fold(column![].spacing(0), |col, (index, track)| { - let is_selected = state.selected_tracks.contains(index); - - let slug_part = text(format!("@{}", track.slug)) - .size(13) - .color(palette.red); - - let title_part = text(format!(" {}", track.title)) - .size(13); - - let track_content = if track.description.is_empty() { - row![slug_part, title_part].spacing(0) - } else { - let desc_part = text(format!(" {}", track.description)) - .size(13) - .color(palette.text_dim); - row![slug_part, title_part, desc_part].spacing(0) - }; - - let item = container(track_content) - .padding(8) - .width(Length::Fill) - .style(move |_theme: &Theme| { - if is_selected { - container::Style { - background: Some(iced::Background::Color(palette.gray3)), - ..Default::default() - } - } else { - container::Style::default() - } - }); - - let button = iced::widget::button(item) - .on_press(Message::TrackClicked(*index)) - .padding(0) - .style(|theme: &Theme, _status| iced::widget::button::Style { - background: None, - text_color: theme.palette().text, - border: iced::Border::default(), - shadow: iced::Shadow::default(), - }); - - let col = col.push(button); - col.push( - horizontal_rule(1).style(move |_theme: &Theme| iced::widget::rule::Style { - color: palette.gray3, - width: 1, - radius: 0.0.into(), - fill_mode: iced::widget::rule::FillMode::Full, - }), - ) - }); - - let content = scrollable(list); - - container(content) - .width(Length::Fill) - .height(Length::Fill) - .into() -} diff --git a/src/tracks_view.rs b/src/tracks_view.rs new file mode 100644 index 0000000..33e1d8e --- /dev/null +++ b/src/tracks_view.rs @@ -0,0 +1,200 @@ +use crate::palette::Palette; +use crate::track::Track; +use crate::view; +use iced::keyboard::Modifiers; +use iced::widget::{column, container, horizontal_rule, row, scrollable, text}; +use iced::{Element, Length, Theme}; + +#[derive(Debug, Clone)] +pub struct State { + pub selected_tracks: Vec, + pub modifiers: Modifiers, +} + +impl Default for State { + fn default() -> Self { + Self { + selected_tracks: Vec::new(), + modifiers: Modifiers::default(), + } + } +} + +#[derive(Debug, Clone)] +pub enum Message { + TrackClicked(usize), + ModifiersChanged(Modifiers), +} + +#[derive(Debug, Clone)] +pub enum Action { + None, +} + +impl State { + pub fn new() -> Self { + Self::default() + } + + pub fn update(&mut self, message: Message) -> Action { + match message { + Message::TrackClicked(index) => { + let ctrl = self.modifiers.command() || self.modifiers.control(); + let shift = self.modifiers.shift(); + + if ctrl { + if self.selected_tracks.contains(&index) { + self.selected_tracks.retain(|&i| i != index); + } else { + self.selected_tracks.push(index); + } + } else if shift { + if let Some(&last) = self.selected_tracks.last() { + let start = last.min(index); + let end = last.max(index); + self.selected_tracks.clear(); + self.selected_tracks.extend(start..=end); + } else { + self.selected_tracks = vec![index]; + } + } else { + self.selected_tracks = vec![index]; + } + Action::None + } + Message::ModifiersChanged(modifiers) => { + self.modifiers = modifiers; + Action::None + } + } + } + + pub fn view<'a>( + &'a self, + all_tracks: &'a [Track], + view_def: &'a view::View, + palette: &'a Palette, + ) -> Element<'a, Message> { + let tracks = sorted_tracks(filtered_tracks(all_tracks, view_def), view_def); + let list = tracks + .iter() + .fold(column![].spacing(0), |col, (index, track)| { + let is_selected = self.selected_tracks.contains(index); + + let slug_part = text(format!("@{}", track.slug)) + .size(13) + .color(palette.red); + + let title_part = text(format!(" {}", track.title)).size(13); + + let track_content = if track.description.is_empty() { + row![slug_part, title_part].spacing(0) + } else { + let desc_part = text(format!(" {}", track.description)) + .size(13) + .color(palette.text_dim); + row![slug_part, title_part, desc_part].spacing(0) + }; + + let item = container(track_content) + .padding(8) + .width(Length::Fill) + .style(move |_theme: &Theme| { + if is_selected { + container::Style { + background: Some(iced::Background::Color(palette.gray3)), + ..Default::default() + } + } else { + container::Style::default() + } + }); + + let button = iced::widget::button(item) + .on_press(Message::TrackClicked(*index)) + .padding(0) + .style(|theme: &Theme, _status| iced::widget::button::Style { + background: None, + text_color: theme.palette().text, + border: iced::Border::default(), + shadow: iced::Shadow::default(), + }); + + let col = col.push(button); + col.push( + horizontal_rule(1).style(move |_theme: &Theme| iced::widget::rule::Style { + color: palette.gray3, + width: 1, + radius: 0.0.into(), + fill_mode: iced::widget::rule::FillMode::Full, + }), + ) + }); + + let content = scrollable(list); + + container(content) + .width(Length::Fill) + .height(Length::Fill) + .into() + } +} + +pub fn filtered_tracks<'a>( + tracks: &'a [Track], + view: &view::View, +) -> Vec<(usize, &'a Track)> { + tracks + .iter() + .enumerate() + .filter(|(_idx, track)| { + if let Some(ref slug) = view.filter.slug { + if &track.slug != slug { + return false; + } + } + + if !view.filter.search_query.is_empty() + && !matches_query(&track.title, &view.filter.search_query) + && !matches_query(&track.description, &view.filter.search_query) + && !matches_query(&track.slug, &view.filter.search_query) + { + return false; + } + + true + }) + .collect() +} + +pub fn sorted_tracks<'a>( + tracks: Vec<(usize, &'a Track)>, + view: &view::View, +) -> Vec<(usize, &'a Track)> { + let mut sorted = tracks; + + match view.sort.field { + view::SortField::Name => { + sorted.sort_by(|(_, a), (_, b)| a.title.cmp(&b.title)); + } + view::SortField::CreatedAt => { + sorted.sort_by(|(_, a), (_, b)| a.created_at.cmp(&b.created_at)); + } + view::SortField::TrackCount | view::SortField::LatestTrack => {} + } + + if matches!(view.sort.direction, view::SortDirection::Desc) { + sorted.reverse(); + } + + sorted.truncate(5000); + + sorted +} + +fn matches_query(text: &str, query: &str) -> bool { + if query.is_empty() { + return true; + } + text.to_lowercase().contains(&query.to_lowercase()) +} diff --git a/todo.txt b/todo.txt index 26e66ad..b3bc182 100644 --- a/todo.txt +++ b/todo.txt @@ -11,6 +11,52 @@ todo → DONE: GPU scrolling verified with 60k items, instant performance (CHANGELOG.md:38) +refactor: scale architecture (iced best practices) +---------------------------------------------------- + +goal: follow iced docs "Scaling Applications" pattern + halloy real-world structure +reason: current flat state/message architecture becoming tangled as app grows + +phase 1: extract view modules with Action pattern + [ ] channels_view.rs + - struct State { selected_channel: Option } + - enum Message { ChannelSelected(usize) } + - enum Action { None, ShowChannelTracks(String, String) } + - fn update(&mut self, msg) -> Action + - fn view(&self, channels: &[Channel]) -> Element + [ ] tracks_view.rs + - struct State { selected_tracks: Vec, modifiers: Modifiers } + - enum Message { TrackClicked(usize), ModifiersChanged(Modifiers) } + - enum Action { None } + - fn update(&mut self, msg) -> Action + - fn view(&self, tracks: &[Track]) -> Element + [ ] update main.rs to route messages, handle Actions + +phase 2: convert PaneContent to view enum + [ ] change PaneContent from view_stack to: + enum PaneContent { + ChannelsView(channels_view::State), + TracksView(tracks_view::State), + ViewEditor(ViewEditor), + } + [ ] each pane contains instantiated view state (not string ID indirection) + [ ] remove views: Vec registry from main State + [ ] view definitions become constructors for view states + +phase 3: separate data from view concerns + [ ] struct DataStore { channels: Vec, tracks: Vec } + [ ] read-only, shared across all views + [ ] view state = selections, scroll, input only + +benefits: +- follows iced docs pattern exactly (Action enum, composable update/view) +- follows halloy Buffer enum pattern (pane content = enum of view types) +- eliminates view ID indirection +- clear separation: data vs view state +- testable view logic (pure update functions) +- type-safe screen transitions via Action + + note ---- when completing tasks, update CHANGELOG.md with what changed -- 2.43.0