diff --git a/crates/ely_app/src/shell/chrome/topbar.rs b/crates/ely_app/src/shell/chrome/topbar.rs index ef5d6d2..8a7fb72 100644 --- a/crates/ely_app/src/shell/chrome/topbar.rs +++ b/crates/ely_app/src/shell/chrome/topbar.rs @@ -3,8 +3,8 @@ use ely_design_system::{colors, spacing}; use ely_domain::{BrowserTab, ThemeMode}; use gpui::{ AnyElement, BoxShadow, Context, FontWeight, InteractiveElement, IntoElement, ParentElement, - SharedString, StatefulInteractiveElement, Styled, div, hsla, point, - prelude::FluentBuilder, px, rgb, rgba, + SharedString, StatefulInteractiveElement, Styled, div, hsla, point, prelude::FluentBuilder, px, + rgb, rgba, }; use gpui_component::{IconName, input::Input}; @@ -27,36 +27,37 @@ pub(crate) fn render_topbar( .flex_shrink_0() .border_b_1() .border_color(rgba(colors::DIVIDER)) - .when(sidebar_collapsed, |el| { - el.child(render_command_bar_identity(snapshot, 56.0, true)) - }) - .child(render_nav_arrow("nav-back", IconName::ArrowLeft)) - .child(render_nav_arrow("nav-forward", IconName::ArrowRight)) + .when(sidebar_collapsed, |el| el.child(render_command_bar_identity(snapshot, 56.0, true))) + .child(render_nav_arrow( + "nav-back", + IconName::ArrowLeft, + active_tab.can_navigate_back(), + cx, + |shell, window, cx| shell.navigate_active_tab_back(window, cx), + )) + .child(render_nav_arrow( + "nav-forward", + IconName::ArrowRight, + active_tab.can_navigate_forward(), + cx, + |shell, window, cx| shell.navigate_active_tab_forward(window, cx), + )) .child(render_omnibar(shell, active_tab, cx)) - .child(render_topbar_action( - "share-url", - IconName::Copy, - cx, - |shell, window, cx| shell.copy_active_tab_url(window, cx), - )) - .child(render_topbar_action( - "open-downloads", - IconName::Folder, - cx, - |shell, window, cx| shell.open_downloads(window, cx), - )) + .child(render_topbar_action("share-url", IconName::Copy, cx, |shell, window, cx| { + shell.copy_active_tab_url(window, cx) + })) + .child(render_topbar_action("open-downloads", IconName::Folder, cx, |shell, window, cx| { + shell.open_downloads(window, cx) + })) .child(render_topbar_action( "toggle-theme", theme_mode_icon(snapshot.appearance.theme_mode()), cx, |shell, _window, cx| shell.cycle_theme_mode(cx), )) - .child(render_topbar_action( - "open-menu", - IconName::Menu, - cx, - |shell, window, cx| shell.open_internal_tab("ely://settings", window, cx), - )) + .child(render_topbar_action("open-menu", IconName::Menu, cx, |shell, window, cx| { + shell.open_internal_tab("ely://settings", window, cx) + })) .into_any_element() } @@ -66,16 +67,11 @@ fn render_omnibar( cx: &mut Context, ) -> AnyElement { let favorite_active = active_tab.flags().favorite; - let favorite_icon = if favorite_active { - IconName::Star - } else { - IconName::StarOff - }; + let favorite_icon = if favorite_active { IconName::Star } else { IconName::StarOff }; let input_value = shell.command_input.read(cx).value().to_string(); let active_url = active_tab.url().as_str().to_string(); - let show_styled = !input_value.is_empty() - && input_value == active_url - && active_url != "ely://new-tab"; + let show_styled = + !input_value.is_empty() && input_value == active_url && active_url != "ely://new-tab"; let secure = active_url.starts_with("https://") || active_url.starts_with("ely://"); div() @@ -123,18 +119,11 @@ fn render_omnibar( } fn render_omnibar_input(shell: &ElyShell) -> AnyElement { - Input::new(&shell.command_input) - .appearance(false) - .cleanable(true) - .into_any_element() + Input::new(&shell.command_input).appearance(false).cleanable(true).into_any_element() } fn render_styled_url(active_tab: &BrowserTab) -> AnyElement { - let host = active_tab - .url() - .host() - .map(|host| host.to_string()) - .unwrap_or_default(); + let host = active_tab.url().host().map(|host| host.to_string()).unwrap_or_default(); let url = active_tab.url().as_str(); let path_start = url.find("://").map(|prefix| prefix + 3).unwrap_or(0); let from_path = &url[path_start..]; @@ -148,32 +137,14 @@ fn render_styled_url(active_tab: &BrowserTab) -> AnyElement { .items_center() .gap(px(2.0)) .text_size(px(13.0)) - .child( - div() - .font_weight(FontWeight(500.0)) - .text_color(rgb(colors::INK)) - .child(host), - ) - .child( - div() - .min_w_0() - .truncate() - .text_color(rgb(colors::INK_3)) - .child(path), - ) + .child(div().font_weight(FontWeight(500.0)).text_color(rgb(colors::INK)).child(host)) + .child(div().min_w_0().truncate().text_color(rgb(colors::INK_3)).child(path)) .into_any_element() } fn render_lock_or_search(secure: bool, show_styled: bool) -> AnyElement { - let icon = if show_styled && !secure { - IconName::Globe - } else { - IconName::Search - }; - div() - .text_color(rgb(colors::INK_3)) - .child(icon) - .into_any_element() + let icon = if show_styled && !secure { IconName::Globe } else { IconName::Search }; + div().text_color(rgb(colors::INK_3)).child(icon).into_any_element() } fn render_omnibar_chip( @@ -203,12 +174,17 @@ where .into_any_element() } -/// Topbar nav arrow placeholder. Per-tab back/forward history is not yet -/// wired through `BrowserCore`, so the buttons render in the design's -/// `disabled` state — visible at INK_5, no hover, no cursor pointer — -/// to honor the "no fake handlers, no mockup" rule. When real history -/// navigation lands the caller can flip these to a clickable variant. -fn render_nav_arrow(id: &'static str, icon: IconName) -> AnyElement { +fn render_nav_arrow( + id: &'static str, + icon: IconName, + enabled: bool, + cx: &mut Context, + handler: F, +) -> AnyElement +where + F: Fn(&mut ElyShell, &mut gpui::Window, &mut Context) + 'static, +{ + let color = if enabled { colors::INK_3 } else { colors::INK_5 }; div() .id(SharedString::from(id)) .size(px(30.0)) @@ -216,7 +192,13 @@ fn render_nav_arrow(id: &'static str, icon: IconName) -> AnyElement { .flex() .items_center() .justify_center() - .text_color(rgb(colors::INK_5)) + .text_color(rgb(color)) + .when(enabled, |el| { + el.cursor_pointer() + .hover(|style| style.bg(rgba(OMNIBAR_BG)).text_color(rgb(colors::INK))) + .active(|style| style.opacity(0.82)) + .on_click(cx.listener(move |shell, _, window, cx| handler(shell, window, cx))) + }) .child(icon) .into_any_element() } diff --git a/crates/ely_app/src/shell/navigation.rs b/crates/ely_app/src/shell/navigation.rs index 9dcaaeb..e2f3a97 100644 --- a/crates/ely_app/src/shell/navigation.rs +++ b/crates/ely_app/src/shell/navigation.rs @@ -78,6 +78,28 @@ impl ElyShell { } } + pub(super) fn navigate_active_tab_back(&mut self, window: &mut Window, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state + && core.navigate_active_tab_back().is_ok_and(|changed| changed) + { + self.sync_address_input(window, cx); + cx.notify(); + } + } + + pub(super) fn navigate_active_tab_forward( + &mut self, + window: &mut Window, + cx: &mut Context, + ) { + if let ShellState::Ready(core) = &mut self.state + && core.navigate_active_tab_forward().is_ok_and(|changed| changed) + { + self.sync_address_input(window, cx); + cx.notify(); + } + } + /// Spawn a fresh tab for `url`. Reserved for "+ New Tab" buttons /// and the deep-link router — anything that explicitly wants a /// new sibling tab rather than navigating in place. @@ -162,9 +184,7 @@ impl ElyShell { return; }; let needle = stripped.trim().to_lowercase(); - let rows = crate::shell::chrome::command_match::visible_command_rows( - &snapshot, &needle, - ); + let rows = crate::shell::chrome::command_match::visible_command_rows(&snapshot, &needle); if rows.is_empty() { return; } diff --git a/crates/ely_browser_core/src/state/tabs.rs b/crates/ely_browser_core/src/state/tabs.rs index 18741b6..071236a 100644 --- a/crates/ely_browser_core/src/state/tabs.rs +++ b/crates/ely_browser_core/src/state/tabs.rs @@ -28,7 +28,7 @@ impl BrowserCore { .iter() .position(|tab| tab.id() == &active_id) .ok_or_else(|| CoreError::TabNotFound { id: active_id.clone() })?; - self.tabs[tab_index].set_url(url); + self.tabs[tab_index].navigate_to(url); self.tabs[tab_index].mark_ready(); let snapshot_tab = self.tabs[tab_index].clone(); self.record_history_entry(&snapshot_tab); @@ -36,6 +36,14 @@ impl BrowserCore { Ok(()) } + pub fn navigate_active_tab_back(&mut self) -> Result { + self.navigate_active_tab_history(TabHistoryDirection::Back) + } + + pub fn navigate_active_tab_forward(&mut self) -> Result { + self.navigate_active_tab_history(TabHistoryDirection::Forward) + } + pub fn open_tab(&mut self, url: UrlText) -> TabId { let tab = self.build_tab(url); let tab_id = tab.id().clone(); @@ -369,6 +377,27 @@ impl BrowserCore { self.tabs.get_mut(active_index).ok_or(CoreError::MissingActiveTab) } + fn navigate_active_tab_history( + &mut self, + direction: TabHistoryDirection, + ) -> Result { + let active_id = self.active_tab_id.clone(); + let tab_index = self.active_tab_index()?; + let navigated_url = match direction { + TabHistoryDirection::Back => self.tabs[tab_index].navigate_back(), + TabHistoryDirection::Forward => self.tabs[tab_index].navigate_forward(), + }; + let Some(_url) = navigated_url else { + return Ok(false); + }; + + self.tabs[tab_index].mark_ready(); + let snapshot_tab = self.tabs[tab_index].clone(); + self.record_history_entry(&snapshot_tab); + self.record_tab_activity(&active_id, SystemTime::now()); + Ok(true) + } + pub(super) fn build_tab(&self, url: UrlText) -> BrowserTab { self.build_tab_for(self.active_space_id.clone(), self.active_profile_id.clone(), url) .with_parent_tab_id(self.active_tab_id.clone()) @@ -425,3 +454,8 @@ impl BrowserCore { ) } } + +enum TabHistoryDirection { + Back, + Forward, +} diff --git a/crates/ely_browser_core/tests/tab_navigation.rs b/crates/ely_browser_core/tests/tab_navigation.rs new file mode 100644 index 0000000..567d049 --- /dev/null +++ b/crates/ely_browser_core/tests/tab_navigation.rs @@ -0,0 +1,53 @@ +use std::error::Error; + +use ely_browser_core::{BrowserCore, InitialBrowserConfig}; +use ely_domain::UrlText; + +#[test] +fn active_tab_tracks_back_and_forward_navigation() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + + core.navigate_active_tab(UrlText::parse("https://example.com/a")?)?; + core.navigate_active_tab(UrlText::parse("https://example.com/b")?)?; + assert!(core.active_tab()?.can_navigate_back()); + assert!(!core.active_tab()?.can_navigate_forward()); + + assert!(core.navigate_active_tab_back()?); + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/a"); + assert!(core.active_tab()?.can_navigate_back()); + assert!(core.active_tab()?.can_navigate_forward()); + + assert!(core.navigate_active_tab_forward()?); + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/b"); + assert!(core.active_tab()?.can_navigate_back()); + assert!(!core.active_tab()?.can_navigate_forward()); + Ok(()) +} + +#[test] +fn new_navigation_clears_forward_stack() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + + core.navigate_active_tab(UrlText::parse("https://example.com/a")?)?; + core.navigate_active_tab(UrlText::parse("https://example.com/b")?)?; + assert!(core.navigate_active_tab_back()?); + + core.navigate_active_tab(UrlText::parse("https://example.com/c")?)?; + + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/c"); + assert!(core.active_tab()?.can_navigate_back()); + assert!(!core.active_tab()?.can_navigate_forward()); + Ok(()) +} + +#[test] +fn empty_history_navigation_keeps_active_url() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let url = core.active_tab()?.url().clone(); + + assert!(!core.navigate_active_tab_back()?); + assert!(!core.navigate_active_tab_forward()?); + + assert_eq!(core.active_tab()?.url(), &url); + Ok(()) +} diff --git a/crates/ely_domain/src/tab.rs b/crates/ely_domain/src/tab.rs index 3da5bfe..b3f0b0c 100644 --- a/crates/ely_domain/src/tab.rs +++ b/crates/ely_domain/src/tab.rs @@ -31,6 +31,8 @@ pub struct BrowserTab { profile_id: ProfileId, title: String, url: UrlText, + back_stack: Vec, + forward_stack: Vec, favicon_key: Option, parent_tab_id: Option, state: TabState, @@ -60,6 +62,8 @@ impl BrowserTab { profile_id, title: title.into(), url, + back_stack: Vec::new(), + forward_stack: Vec::new(), favicon_key: None, parent_tab_id: None, state: TabState::Ready, @@ -116,6 +120,16 @@ impl BrowserTab { &self.url } + #[must_use] + pub fn can_navigate_back(&self) -> bool { + !self.back_stack.is_empty() + } + + #[must_use] + pub fn can_navigate_forward(&self) -> bool { + !self.forward_stack.is_empty() + } + #[must_use] pub fn favicon_key(&self) -> Option<&str> { self.favicon_key.as_deref() @@ -271,15 +285,39 @@ impl BrowserTab { self.sync_enabled = sync_enabled; } - /// Replace this tab's URL in place. Used for in-tab navigation - /// (clicking a link, picking a settings sub-page, etc.) where the - /// active tab should follow the user instead of spawning a new - /// one for every URL change. Title stays as set; the caller can - /// re-derive it from the new URL if it wants to. + /// Replace this tab's URL directly while preserving navigation + /// stacks. Title stays as set; callers can re-derive it from the + /// new URL when needed. pub fn set_url(&mut self, url: UrlText) { self.url = url; self.last_active_at = SystemTime::now(); } + + /// Navigate in place and record the previous URL in this tab's + /// back stack. + pub fn navigate_to(&mut self, url: UrlText) { + if self.url != url { + self.back_stack.push(self.url.clone()); + self.forward_stack.clear(); + } + self.set_url(url); + } + + pub fn navigate_back(&mut self) -> Option { + let target = self.back_stack.pop()?; + let current = std::mem::replace(&mut self.url, target); + self.forward_stack.push(current); + self.last_active_at = SystemTime::now(); + Some(self.url.clone()) + } + + pub fn navigate_forward(&mut self) -> Option { + let target = self.forward_stack.pop()?; + let current = std::mem::replace(&mut self.url, target); + self.back_stack.push(current); + self.last_active_at = SystemTime::now(); + Some(self.url.clone()) + } } pub fn validate_zoom_percent(value: u16) -> Result {