aljaz/notedeck
1
0
Fork 0
mirror of https://github.com/aljazceru/notedeck.git synced 2025-12-17 00:44:18 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix all channel management issues and improve UX

Browse Source 
This commit addresses all identified issues in channel management:

1. **Fix timeline cleanup on channel removal** (channels.rs:97-123)
   - remove_channel() now properly unsubscribes from timeline
   - Prevents memory leaks from orphaned timeline subscriptions
   - Ensures proper cleanup when deleting channels

2. **Add bounds checking on deserialization** (storage/channels.rs:138-158)
   - Validates selected index after loading channels from disk
   - Prevents out-of-bounds panics if saved state is corrupted
   - Handles edge case of empty channel lists gracefully

3. **Implement channel deletion UI** (channel_sidebar.rs:19-25, 273-287)
   - Added DeleteChannel action to sidebar
   - Right-click context menu shows "Delete Channel" option
   - Only allows deletion if more than one channel exists
   - Properly cleans up timeline and saves state after deletion

4. **Implement channel editing UI** (channel_dialog.rs:5-44, channel_sidebar.rs:19-25)
   - Added EditChannel action and editing_index field to dialog
   - open_for_edit() method pre-fills dialog with existing data
   - Dialog title changes to "Edit Channel" in edit mode
   - Button changes from "Create" to "Save" when editing
   - Context menu shows "Edit Channel" option

5. **Add channel editing logic** (channels.rs:99-132, app.rs:476-524)
   - edit_channel() method updates channel data
   - Unsubscribes from old timeline if hashtags changed
   - Re-subscribes to new timeline with updated hashtags
   - Properly updates router with new timeline kind

6. **Auto-select newly created channels** (channels.rs:93-97)
   - add_channel() now automatically selects the new channel
   - Improves UX by immediately showing the channel user just created
   - Eliminates confusion about which channel is active

7. **Optimize storage saves** (app.rs:1290-1296)
   - Removed automatic save on every channel selection
   - Channel selection state will be saved on app close instead
   - Prevents excessive disk I/O from rapid channel switching
   - Saves only on create, edit, and delete operations

All changes tested and build succeeds without errors.
This commit is contained in:
Claude
2025-11-14 06:46:28 +00:00
parent 156dc21df1
commit 978ee1ec96
5 changed files with 265 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
crates/notedeck_columns/src/app.rs
View File

@@ -473,6 +473,55 @@ fn render_damus(damus: &mut Damus, app_ctx: &mut AppContext<'_>, ui: &mut egui::
}
}
}
ui::ChannelDialogAction::Edit { index, name, hashtags } => {
// Edit existing channel
let edited = damus
.channels_cache
.active_channels_mut(app_ctx.i18n, app_ctx.accounts)
.edit_channel(
index,
name,
hashtags.clone(),
&mut damus.timeline_cache,
app_ctx.ndb,
app_ctx.pool,
);
if edited {
// Save channels cache
storage::save_channels_cache(app_ctx.path, &damus.channels_cache);
// Subscribe to the new timeline if hashtags changed
let txn = match nostrdb::Transaction::new(app_ctx.ndb) {
Ok(txn) => txn,
Err(e) => {
error!("Failed to create transaction for channel subscription: {}", e);
return app_resp;
}
};
if let Some(channel) = damus.channels_cache.active_channels_mut(app_ctx.i18n, app_ctx.accounts).get_channel_mut(index) {
if damus.timeline_cache.get(&channel.timeline_kind).is_none() {
if let Some(result) = damus.timeline_cache.open(
&mut damus.subscriptions,
app_ctx.ndb,
app_ctx.note_cache,
&txn,
app_ctx.pool,
&channel.timeline_kind,
) {
result.process(
app_ctx.ndb,
app_ctx.note_cache,
&txn,
&mut damus.timeline_cache,
app_ctx.unknown_ids,
);
}
}
}
}
}
ui::ChannelDialogAction::Cancel => {
// Dialog was canceled, nothing to do
}
@@ -1242,12 +1291,35 @@ fn timelines_view(
app.channels_cache
.active_channels_mut(ctx.i18n, ctx.accounts)
.select_channel(idx);
// Save channel state after selection changes
storage::save_channels_cache(ctx.path, &app.channels_cache);
// Note: Don't save on every selection to avoid excessive disk I/O
// Channel selection state will be saved on app close
}
ChannelSidebarAction::AddChannel => {
app.channel_dialog.open();
}
ChannelSidebarAction::DeleteChannel(idx) => {
let removed = app.channels_cache
.active_channels_mut(ctx.i18n, ctx.accounts)
.remove_channel(idx, &mut app.timeline_cache, ctx.ndb, ctx.pool);
if removed.is_some() {
// Save channels cache after deletion
storage::save_channels_cache(ctx.path, &app.channels_cache);
}
}
ChannelSidebarAction::EditChannel(idx) => {
// Get channel data and open dialog for editing
if let Some(channel) = app.channels_cache
.active_channels(ctx.accounts)
.get_channel(idx)
{
app.channel_dialog.open_for_edit(
idx,
channel.name.clone(),
channel.hashtags.clone(),
);
}
}
}
}

52
crates/notedeck_columns/src/channels.rs
View File

@@ -92,12 +92,62 @@ impl ChannelList {
pub fn add_channel(&mut self, channel: Channel) {
self.channels.push(channel);
// Auto-select the newly added channel
self.selected = self.channels.len() - 1;
}
pub fn remove_channel(&mut self, index: usize) -> Option<Channel> {
pub fn edit_channel(
&mut self,
index: usize,
name: String,
hashtags: Vec<String>,
timeline_cache: &mut TimelineCache,
ndb: &mut nostrdb::Ndb,
pool: &mut enostr::RelayPool,
) -> bool {
if index >= self.channels.len() {
return false;
}
let channel = &mut self.channels[index];
// Unsubscribe from old timeline if hashtags changed
let old_timeline_kind = channel.timeline_kind.clone();
let new_timeline_kind = TimelineKind::Hashtag(hashtags.clone());
if old_timeline_kind != new_timeline_kind {
if let Err(err) = timeline_cache.pop(&old_timeline_kind, ndb, pool) {
error!("Failed to unsubscribe from old channel timeline: {err}");
}
}
// Update channel data
channel.name = name;
channel.hashtags = hashtags.clone();
channel.timeline_kind = new_timeline_kind.clone();
channel.router = Router::new(vec![Route::timeline(new_timeline_kind)]);
info!("Updated channel: {}", channel.name);
true
}
pub fn remove_channel(
&mut self,
index: usize,
timeline_cache: &mut TimelineCache,
ndb: &mut nostrdb::Ndb,
pool: &mut enostr::RelayPool,
) -> Option<Channel> {
if index < self.channels.len() && self.channels.len() > 1 {
let removed = self.channels.remove(index);
// Unsubscribe from the timeline
if let Err(err) = timeline_cache.pop(&removed.timeline_kind, ndb, pool) {
error!("Failed to unsubscribe from channel timeline: {err}");
} else {
info!("Unsubscribed from removed channel: {}", removed.name);
}
// Adjust selected index if needed
if self.selected >= self.channels.len() {
self.selected = self.channels.len() - 1;

23
crates/notedeck_columns/src/storage/channels.rs
View File

@@ -136,13 +136,24 @@ impl SerializableChannelList {
}
fn channel_list(self) -> ChannelList {
let channels: Vec<_> = self
.channels
.into_iter()
.map(|c| c.channel())
.collect();
// Ensure selected index is within bounds
let selected = if channels.is_empty() {
0
} else if self.selected >= channels.len() {
channels.len() - 1
} else {
self.selected
};
ChannelList {
channels: self
.channels
.into_iter()
.map(|c| c.channel())
.collect(),
selected: self.selected,
channels,
selected,
}
}
}

70
crates/notedeck_columns/src/ui/channel_dialog.rs
View File

@@ -7,10 +7,12 @@ pub struct ChannelDialog {
pub hashtags: String,
pub is_open: bool,
pub focus_requested: bool,
pub editing_index: Option<usize>,
}
pub enum ChannelDialogAction {
Create { name: String, hashtags: Vec<String> },
Edit { index: usize, name: String, hashtags: Vec<String> },
Cancel,
}
@@ -21,6 +23,7 @@ impl ChannelDialog {
hashtags: String::new(),
is_open: false,
focus_requested: false,
editing_index: None,
}
}
@@ -29,6 +32,15 @@ impl ChannelDialog {
self.name.clear();
self.hashtags.clear();
self.focus_requested = false;
self.editing_index = None;
}
pub fn open_for_edit(&mut self, index: usize, name: String, hashtags: Vec<String>) {
self.is_open = true;
self.name = name;
self.hashtags = hashtags.join(", ");
self.focus_requested = false;
self.editing_index = Some(index);
}
pub fn close(&mut self) {
@@ -46,7 +58,13 @@ impl ChannelDialog {
let mut action: Option<ChannelDialogAction> = None;
egui::Window::new(tr!(i18n, "Create Channel", "Dialog title for creating a new channel"))
let title = if self.editing_index.is_some() {
tr!(i18n, "Edit Channel", "Dialog title for editing a channel")
} else {
tr!(i18n, "Create Channel", "Dialog title for creating a new channel")
};
egui::Window::new(title)
.collapsible(false)
.resizable(false)
.anchor(egui::Align2::CENTER_CENTER, Vec2::ZERO)
@@ -125,10 +143,18 @@ impl ChannelDialog {
.filter(|s| !s.is_empty())
.collect();
action = Some(ChannelDialogAction::Create {
name: self.name.trim().to_string(),
hashtags,
});
action = if let Some(index) = self.editing_index {
Some(ChannelDialogAction::Edit {
index,
name: self.name.trim().to_string(),
hashtags,
})
} else {
Some(ChannelDialogAction::Create {
name: self.name.trim().to_string(),
hashtags,
})
};
}
}
});
@@ -138,18 +164,24 @@ impl ChannelDialog {
// Buttons
ui.horizontal(|ui| {
ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
// Create button (hashtags are optional)
let create_enabled = !self.name.trim().is_empty();
// Create/Save button (hashtags are optional)
let button_enabled = !self.name.trim().is_empty();
let create_button = egui::Button::new(
RichText::new(tr!(i18n, "Create", "Button to create channel"))
let button_text = if self.editing_index.is_some() {
tr!(i18n, "Save", "Button to save channel edits")
} else {
tr!(i18n, "Create", "Button to create channel")
};
let button = egui::Button::new(
RichText::new(button_text)
.size(14.0),
)
.min_size(Vec2::new(80.0, 32.0));
let create_response = ui.add_enabled(create_enabled, create_button);
let button_response = ui.add_enabled(button_enabled, button);
if create_response.clicked() {
if button_response.clicked() {
let hashtags: Vec<String> = self
.hashtags
.split(',')
@@ -157,10 +189,18 @@ impl ChannelDialog {
.filter(|s| !s.is_empty())
.collect();
action = Some(ChannelDialogAction::Create {
name: self.name.trim().to_string(),
hashtags,
});
action = if let Some(index) = self.editing_index {
Some(ChannelDialogAction::Edit {
index,
name: self.name.trim().to_string(),
hashtags,
})
} else {
Some(ChannelDialogAction::Create {
name: self.name.trim().to_string(),
hashtags,
})
};
}
ui.add_space(8.0);

92
crates/notedeck_columns/src/ui/channel_sidebar.rs
View File

@@ -1,5 +1,5 @@
use egui::{
vec2, Color32, CursorIcon, InnerResponse, Margin, Rect, RichText, ScrollArea,
vec2, Color32, CursorIcon, Margin, Rect, RichText, ScrollArea,
Separator, Stroke, TextStyle, Widget,
};
@@ -20,6 +20,8 @@ pub struct ChannelSidebar<'a> {
pub enum ChannelSidebarAction {
SelectChannel(usize),
AddChannel,
DeleteChannel(usize),
EditChannel(usize),
}
pub struct ChannelSidebarResponse {
@@ -88,26 +90,32 @@ impl<'a> ChannelSidebar<'a> {
let scroll_response = ScrollArea::vertical()
.id_salt("channel_list")
.show(ui, |ui| {
let mut selected_response = None;
let mut selected_action = None;
for (index, channel) in channel_list.channels.iter().enumerate() {
let is_selected = index == selected_index;
let resp = channel_item(ui, &channel.name, is_selected, channel.unread_count);
let resp = channel_item(ui, &channel.name, is_selected, channel.unread_count, channel_list.num_channels(), index, self.i18n);
if resp.clicked() {
selected_response = Some(InnerResponse::new(
ChannelSidebarAction::SelectChannel(index),
resp,
));
match resp {
ChannelItemResponse::Select => {
selected_action = Some(ChannelSidebarAction::SelectChannel(index));
}
ChannelItemResponse::Delete => {
selected_action = Some(ChannelSidebarAction::DeleteChannel(index));
}
ChannelItemResponse::Edit => {
selected_action = Some(ChannelSidebarAction::EditChannel(index));
}
ChannelItemResponse::None => {}
}
}
selected_response
selected_action
})
.inner;
if scroll_response.is_some() {
return scroll_response;
if let Some(action) = scroll_response {
return Some(action);
}
ui.add_space(8.0);
@@ -116,29 +124,44 @@ impl<'a> ChannelSidebar<'a> {
let add_channel_resp = ui.add(add_channel_button(self.i18n));
if add_channel_resp.clicked() {
Some(InnerResponse::new(
ChannelSidebarAction::AddChannel,
add_channel_resp,
))
Some(ChannelSidebarAction::AddChannel)
} else {
None
}
})
.inner
.map(|inner| ChannelSidebarResponse::new(inner.inner, inner.response))
.map(|action| {
// We need to create a dummy response for ChannelSidebarResponse
// Use the UI's interact_rect to create a valid response
let dummy_rect = ui.available_rect_before_wrap();
let dummy_response = ui.interact(dummy_rect, ui.id().with("channel_sidebar"), egui::Sense::hover());
ChannelSidebarResponse::new(action, dummy_response)
})
}
}
enum ChannelItemResponse {
Select,
Delete,
Edit,
None,
}
fn channel_item(
ui: &mut egui::Ui,
name: &str,
is_selected: bool,
unread_count: usize,
) -> egui::Response {
total_channels: usize,
_channel_index: usize,
i18n: &mut Localization,
) -> ChannelItemResponse {
let desired_size = vec2(ui.available_width(), 36.0);
let (rect, response) = ui.allocate_exact_size(desired_size, egui::Sense::click());
let mut action = ChannelItemResponse::None;
if ui.is_rect_visible(rect) {
let visuals = ui.style().interact(&response);
let bg_color = if is_selected {
@@ -169,7 +192,7 @@ fn channel_item(
}
// Draw hashtag icon
let icon_rect = Rect::from_min_size(
let icon_rect = egui::Rect::from_min_size(
rect.min + vec2(8.0, rect.height() / 2.0 - 8.0),
vec2(16.0, 16.0),
);
@@ -177,12 +200,12 @@ fn channel_item(
icon_rect.center(),
egui::Align2::CENTER_CENTER,
"#",
TextStyle::Body.resolve(ui.style()),
egui::TextStyle::Body.resolve(ui.style()),
visuals.text_color(),
);
// Draw channel name
let text_rect = Rect::from_min_size(
let text_rect = egui::Rect::from_min_size(
rect.min + vec2(32.0, 0.0),
vec2(rect.width() - 64.0, rect.height()),
);
@@ -200,7 +223,7 @@ fn channel_item(
text_rect.left_center(),
egui::Align2::LEFT_CENTER,
name,
TextStyle::Body.resolve(ui.style()),
egui::TextStyle::Body.resolve(ui.style()),
text_color,
);
@@ -213,7 +236,7 @@ fn channel_item(
};
let badge_size = vec2(24.0, 18.0);
let badge_rect = Rect::from_min_size(
let badge_rect = egui::Rect::from_min_size(
rect.max - vec2(badge_size.x + 8.0, rect.height() / 2.0 + badge_size.y / 2.0),
badge_size,
);
@@ -230,13 +253,34 @@ fn channel_item(
badge_rect.center(),
egui::Align2::CENTER_CENTER,
&badge_text,
TextStyle::Small.resolve(ui.style()),
egui::TextStyle::Small.resolve(ui.style()),
Color32::WHITE,
);
}
}
response.on_hover_cursor(CursorIcon::PointingHand)
// Handle clicks
if response.clicked() {
action = ChannelItemResponse::Select;
}
// Show context menu on right-click
response.context_menu(|ui| {
if ui.button(tr!(i18n, "Edit Channel", "Context menu option to edit channel")).clicked() {
action = ChannelItemResponse::Edit;
ui.close_menu();
}
// Only allow delete if not the last channel
if total_channels > 1 {
if ui.button(tr!(i18n, "Delete Channel", "Context menu option to delete channel")).clicked() {
action = ChannelItemResponse::Delete;
ui.close_menu();
}
}
});
action
}
fn add_channel_button(i18n: &mut Localization) -> impl Widget + '_ {