Add bookmark metadata editor

crates/ely_app/src/shell/bookmarks.rs

@@ -0,0 +1,125 @@
+use ely_domain::{BookmarkEntry, BookmarkId};
+use gpui::{AppContext, Context, Entity, Window};
+use gpui_component::input::InputState;
+
+use super::{ElyShell, ShellState};
+
+#[derive(Clone)]
+pub(super) struct PendingBookmarkEdit {
+    bookmark_id: BookmarkId,
+    collection_input: Entity<InputState>,
+    tags_input: Entity<InputState>,
+    note_input: Entity<InputState>,
+}
+
+impl PendingBookmarkEdit {
+    fn new(bookmark: &BookmarkEntry, window: &mut Window, cx: &mut Context<ElyShell>) -> Self {
+        let collection_input =
+            bookmark_input(bookmark.collection_name().to_string(), "Collection", window, cx);
+        let tags_input = bookmark_input(bookmark.tags().join(", "), "Tags", window, cx);
+        let note_input =
+            bookmark_input(bookmark.note().unwrap_or_default().to_string(), "Note", window, cx);
+
+        Self { bookmark_id: bookmark.id().clone(), collection_input, tags_input, note_input }
+    }
+
+    pub(super) fn bookmark_id(&self) -> &BookmarkId {
+        &self.bookmark_id
+    }
+
+    pub(super) fn collection_input(&self) -> &Entity<InputState> {
+        &self.collection_input
+    }
+
+    pub(super) fn tags_input(&self) -> &Entity<InputState> {
+        &self.tags_input
+    }
+
+    pub(super) fn note_input(&self) -> &Entity<InputState> {
+        &self.note_input
+    }
+
+    pub(super) fn matches(&self, bookmark_id: &BookmarkId) -> bool {
+        &self.bookmark_id == bookmark_id
+    }
+
+    fn values(&self, cx: &mut Context<ElyShell>) -> (String, Vec<String>, Option<String>) {
+        (
+            self.collection_input.read(cx).value().to_string(),
+            parse_bookmark_tags(&self.tags_input.read(cx).value()),
+            optional_bookmark_note(&self.note_input.read(cx).value()),
+        )
+    }
+
+    fn focus_collection(&self, window: &mut Window, cx: &mut Context<ElyShell>) {
+        self.collection_input.update(cx, |input, cx| {
+            input.focus(window, cx);
+        });
+    }
+}
+
+impl ElyShell {
+    pub(super) fn start_bookmark_edit(
+        &mut self,
+        bookmark: &BookmarkEntry,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let pending_edit = PendingBookmarkEdit::new(bookmark, window, cx);
+        pending_edit.focus_collection(window, cx);
+        self.pending_bookmark_edit = Some(pending_edit);
+        self.bookmark_edit_error = None;
+        cx.notify();
+    }
+
+    pub(super) fn cancel_bookmark_edit(&mut self, cx: &mut Context<Self>) {
+        self.pending_bookmark_edit = None;
+        self.bookmark_edit_error = None;
+        cx.notify();
+    }
+
+    pub(super) fn save_bookmark_edit(&mut self, cx: &mut Context<Self>) {
+        let Some(pending_edit) = self.pending_bookmark_edit.clone() else {
+            return;
+        };
+        let (collection_name, tags, note) = pending_edit.values(cx);
+
+        let result = match &mut self.state {
+            ShellState::Ready(core) => core
+                .update_bookmark_metadata(pending_edit.bookmark_id(), collection_name, tags, note)
+                .map_err(|error| error.to_string()),
+            ShellState::StartupError(message) => Err(message.clone()),
+        };
+
+        self.bookmark_edit_error = result.err();
+        if self.bookmark_edit_error.is_none() {
+            self.pending_bookmark_edit = None;
+        }
+        cx.notify();
+    }
+}
+
+fn bookmark_input(
+    value: String,
+    placeholder: &'static str,
+    window: &mut Window,
+    cx: &mut Context<ElyShell>,
+) -> Entity<InputState> {
+    cx.new(move |cx| InputState::new(window, cx).placeholder(placeholder).default_value(value))
+}
+
+fn parse_bookmark_tags(value: &str) -> Vec<String> {
+    value.split(',').filter_map(normalized_text).collect()
+}
+
+fn optional_bookmark_note(value: &str) -> Option<String> {
+    normalized_text(value)
+}
+
+fn normalized_text(value: &str) -> Option<String> {
+    let trimmed = value.trim();
+    if trimmed.is_empty() {
+        return None;
+    }
+    Some(trimmed.to_string())
+}

crates/ely_app/src/shell/internal_pages/bookmarks.rs

@@ -2,10 +2,15 @@ use ely_browser_core::BrowserSnapshot;
 use ely_design_system::colors;
 use ely_domain::BookmarkEntry;
 use gpui::{
-    AnyElement, Context, InteractiveElement, IntoElement, ParentElement, SharedString,
+    AnyElement, Context, Entity, InteractiveElement, IntoElement, ParentElement, SharedString,
-    StatefulInteractiveElement, Styled, div, px, rgb,
+    StatefulInteractiveElement, Styled, div, prelude::FluentBuilder, px, rgb,
+};
+use gpui_component::{
+    IconName, Selectable, Sizable, StyledExt,
+    button::{Button, ButtonVariants},
+    input::{Input, InputState},
+    scroll::ScrollableElement,
 };
-use gpui_component::{IconName, StyledExt, scroll::ScrollableElement};
 
 use super::{ElyShell, render_canvas_surface};
 
@@ -57,17 +62,27 @@ impl ElyShell {
                     .bookmarks
                     .iter()
                     .rev()
-                    .map(|bookmark| self.render_bookmark_row(bookmark, cx)),
+                    .enumerate()
+                    .map(|(index, bookmark)| self.render_bookmark_row(index, bookmark, cx)),
             )
             .into_any_element()
     }
 
     fn render_bookmark_row(
         &mut self,
+        index: usize,
         bookmark: &BookmarkEntry,
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let url = bookmark.url().clone();
+        let open_url = bookmark.url().clone();
+        let edit_bookmark = bookmark.clone();
+        let pending_edit = self
+            .pending_bookmark_edit
+            .clone()
+            .filter(|pending_edit| pending_edit.matches(bookmark.id()));
+        let editing = pending_edit.is_some();
+        let edit_error = self.bookmark_edit_error.clone();
 
         div()
             .id(SharedString::from(format!("bookmark-{}", bookmark.id().as_str())))
@@ -75,58 +90,167 @@ impl ElyShell {
             .border_b_1()
             .border_color(rgb(colors::HAIRLINE))
             .flex()
-            .items_center()
+            .flex_col()
-            .justify_between()
+            .gap_3()
-            .gap_4()
-            .cursor_pointer()
-            .hover(|style| style.bg(rgb(colors::CANVAS_SOFT)))
-            .active(|style| style.opacity(0.82))
-            .on_click(cx.listener(move |shell, _, window, cx| {
-                shell.open_url(url.clone(), window, cx);
-            }))
             .child(
                 div()
-                    .min_w_0()
                     .flex()
                     .items_center()
-                    .gap_3()
+                    .justify_between()
-                    .child(div().text_color(rgb(colors::MUTED_SOFT)).child(IconName::BookOpen))
+                    .gap_4()
                     .child(
                         div()
                             .min_w_0()
+                            .id(SharedString::from(format!(
+                                "bookmark-open-{}",
+                                bookmark.id().as_str()
+                            )))
+                            .flex_1()
                             .flex()
-                            .flex_col()
+                            .items_center()
-                            .gap_1()
+                            .gap_3()
+                            .cursor_pointer()
+                            .hover(|style| style.bg(rgb(colors::CANVAS_SOFT)))
+                            .on_click(cx.listener(move |shell, _, window, cx| {
+                                shell.open_url(url.clone(), window, cx);
+                            }))
                             .child(
-                                div()
+                                div().text_color(rgb(colors::MUTED_SOFT)).child(IconName::BookOpen),
-                                    .text_sm()
+                            )
-                                    .font_semibold()
+                            .child(render_bookmark_summary(bookmark)),
-                                    .truncate()
+                    )
-                                    .text_color(rgb(colors::INK))
+                    .child(
-                                    .child(bookmark.title().to_string()),
+                        div()
+                            .flex()
+                            .items_center()
+                            .gap_2()
+                            .child(
+                                Button::new(("open-bookmark", index))
+                                    .ghost()
+                                    .xsmall()
+                                    .icon(IconName::ExternalLink)
+                                    .tooltip("Open Bookmark")
+                                    .on_click(cx.listener(move |shell, _, window, cx| {
+                                        shell.open_url(open_url.clone(), window, cx);
+                                    })),
                             )
                             .child(
-                                div()
+                                Button::new(("edit-bookmark", index))
-                                    .text_xs()
+                                    .ghost()
-                                    .truncate()
+                                    .xsmall()
-                                    .text_color(rgb(colors::MUTED))
+                                    .selected(editing)
-                                    .child(bookmark.display_url()),
+                                    .icon(IconName::Replace)
+                                    .tooltip("Edit Bookmark")
+                                    .on_click(cx.listener(move |shell, _, window, cx| {
+                                        shell.start_bookmark_edit(&edit_bookmark, window, cx);
+                                    })),
                             ),
                     ),
             )
-            .child(
+            .when_some(pending_edit, |this, pending_edit| {
-                div()
+                this.child(render_bookmark_editor(index, &pending_edit, edit_error.as_deref(), cx))
-                    .max_w(px(180.0))
+            })
-                    .truncate()
-                    .text_xs()
-                    .font_semibold()
-                    .text_color(rgb(colors::MUTED))
-                    .child(bookmark.collection_name().to_string()),
-            )
             .into_any_element()
     }
 }
 
+fn render_bookmark_summary(bookmark: &BookmarkEntry) -> AnyElement {
+    div()
+        .min_w_0()
+        .flex()
+        .flex_col()
+        .gap_1()
+        .child(
+            div()
+                .text_sm()
+                .font_semibold()
+                .truncate()
+                .text_color(rgb(colors::INK))
+                .child(bookmark.title().to_string()),
+        )
+        .child(
+            div().text_xs().truncate().text_color(rgb(colors::MUTED)).child(bookmark.display_url()),
+        )
+        .child(
+            div()
+                .text_xs()
+                .truncate()
+                .text_color(rgb(colors::MUTED_SOFT))
+                .child(bookmark_metadata_label(bookmark)),
+        )
+        .into_any_element()
+}
+
+fn render_bookmark_editor(
+    index: usize,
+    pending_edit: &crate::shell::bookmarks::PendingBookmarkEdit,
+    edit_error: Option<&str>,
+    cx: &mut Context<ElyShell>,
+) -> AnyElement {
+    div()
+        .rounded_md()
+        .border_1()
+        .border_color(rgb(colors::HAIRLINE_STRONG))
+        .bg(rgb(colors::CANVAS_SOFT))
+        .p_3()
+        .flex()
+        .flex_col()
+        .gap_3()
+        .child(
+            div()
+                .flex()
+                .gap_3()
+                .child(render_bookmark_edit_field("Collection", pending_edit.collection_input()))
+                .child(render_bookmark_edit_field("Tags", pending_edit.tags_input())),
+        )
+        .child(render_bookmark_edit_field("Note", pending_edit.note_input()))
+        .when_some(edit_error.map(ToOwned::to_owned), |this, message| {
+            this.child(div().text_xs().text_color(rgb(colors::ERROR)).child(message))
+        })
+        .child(
+            div()
+                .flex()
+                .items_center()
+                .justify_end()
+                .gap_2()
+                .child(
+                    Button::new(("cancel-bookmark-edit", index))
+                        .ghost()
+                        .xsmall()
+                        .icon(IconName::Close)
+                        .label("Cancel")
+                        .tooltip("Cancel Bookmark Edit")
+                        .on_click(cx.listener(|shell, _, _, cx| {
+                            shell.cancel_bookmark_edit(cx);
+                        })),
+                )
+                .child(
+                    Button::new(("save-bookmark-edit", index))
+                        .primary()
+                        .xsmall()
+                        .icon(IconName::Check)
+                        .label("Save")
+                        .tooltip("Save Bookmark")
+                        .on_click(cx.listener(|shell, _, _, cx| {
+                            shell.save_bookmark_edit(cx);
+                        })),
+                ),
+        )
+        .into_any_element()
+}
+
+fn render_bookmark_edit_field(label: &'static str, input: &Entity<InputState>) -> AnyElement {
+    div()
+        .min_w_0()
+        .flex_1()
+        .flex()
+        .flex_col()
+        .gap_1()
+        .child(div().text_xs().font_semibold().text_color(rgb(colors::MUTED)).child(label))
+        .child(Input::new(input).small())
+        .into_any_element()
+}
+
 fn render_bookmarks_header(snapshot: &BrowserSnapshot) -> AnyElement {
     div()
         .flex()
@@ -154,6 +278,17 @@ fn render_bookmarks_header(snapshot: &BrowserSnapshot) -> AnyElement {
         .into_any_element()
 }
 
+fn bookmark_metadata_label(bookmark: &BookmarkEntry) -> String {
+    let mut parts = vec![bookmark.collection_name().to_string()];
+    if !bookmark.tags().is_empty() {
+        parts.push(format!("Tags: {}", bookmark.tags().join(", ")));
+    }
+    if let Some(note) = bookmark.note() {
+        parts.push(note.to_string());
+    }
+    parts.join(" - ")
+}
+
 fn bookmark_count_label(count: usize) -> String {
     match count {
         1 => "1 bookmark".to_string(),

crates/ely_app/src/shell/mod.rs

@@ -1,3 +1,4 @@
+mod bookmarks;
 mod downloads;
 mod history;
 mod internal_pages;
@@ -15,6 +16,7 @@ use ely_domain::{
 use gpui::{App, AppContext, Context, Entity, FocusHandle, Focusable, Subscription, Window};
 use gpui_component::input::{InputEvent, InputState, SelectAll};
 
+use bookmarks::PendingBookmarkEdit;
 use downloads::PendingDownloadFileAction;
 use history::{PendingHistoryDomainClear, PendingHistoryTimeClear};
 use plugins::{PendingPluginInstall, PendingPluginUninstall};
@@ -42,6 +44,8 @@ pub struct ElyShell {
     pending_history_domain_clear: Option<PendingHistoryDomainClear>,
     pending_history_time_clear: Option<PendingHistoryTimeClear>,
     site_permissions_clear_confirmation: Option<ProfileId>,
+    pending_bookmark_edit: Option<PendingBookmarkEdit>,
+    bookmark_edit_error: Option<String>,
     plugin_install_error: Option<String>,
     pending_plugin_install: Option<PendingPluginInstall>,
     pending_plugin_uninstall: Option<PendingPluginUninstall>,
@@ -107,6 +111,8 @@ impl ElyShell {
             pending_history_domain_clear: None,
             pending_history_time_clear: None,
             site_permissions_clear_confirmation: None,
+            pending_bookmark_edit: None,
+            bookmark_edit_error: None,
             plugin_install_error: None,
             pending_plugin_install: None,
             pending_plugin_uninstall: None,

crates/ely_browser_core/src/state/bookmarks.rs

@@ -63,6 +63,28 @@ impl BrowserCore {
         Ok(())
     }
 
+    pub fn update_bookmark_metadata(
+        &mut self,
+        bookmark_id: &BookmarkId,
+        collection_name: impl Into<String>,
+        tags: Vec<String>,
+        note: Option<String>,
+    ) -> Result<(), CoreError> {
+        let bookmark = self.bookmark_mut(bookmark_id)?;
+        let mut updated_bookmark = bookmark.clone();
+
+        updated_bookmark.set_collection_name(collection_name)?;
+        updated_bookmark.set_tags(tags)?;
+        if let Some(note) = note {
+            updated_bookmark.set_note(note)?;
+        } else {
+            updated_bookmark.clear_note();
+        }
+
+        *bookmark = updated_bookmark;
+        Ok(())
+    }
+
     pub(super) fn find_bookmark_match(&self, query: &str) -> Option<UrlText> {
         let normalized_query = query.trim().to_lowercase();
         if normalized_query.is_empty() {

crates/ely_browser_core/tests/bookmarks.rs

@@ -65,6 +65,29 @@ fn bookmark_metadata_updates_collection_tags_and_note() -> Result<(), Box<dyn Er
     Ok(())
 }
 
+#[test]
+fn bookmark_metadata_batch_update_is_atomic() -> Result<(), Box<dyn Error>> {
+    let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
+    core.open_tab(UrlText::parse("https://example.com/research")?);
+    let bookmark_id = core.bookmark_active_tab()?;
+
+    let Err(error) = core.update_bookmark_metadata(
+        &bookmark_id,
+        "Research",
+        vec!["rust".to_string(), " ".to_string()],
+        Some("Read later".to_string()),
+    ) else {
+        return Err("expected invalid bookmark metadata error".into());
+    };
+    let snapshot = core.snapshot()?;
+
+    assert_eq!(error, CoreError::Domain(DomainError::EmptyField { field: "bookmark tag" }));
+    assert_eq!(snapshot.bookmarks[0].collection_name(), "Work");
+    assert!(snapshot.bookmarks[0].tags().is_empty());
+    assert_eq!(snapshot.bookmarks[0].note(), None);
+    Ok(())
+}
+
 #[test]
 fn bookmark_metadata_rejects_empty_fields() -> Result<(), Box<dyn Error>> {
     let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;