From f8e05604edc85d5e1657ff4ea805b667dcd48236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 9 May 2026 22:42:25 -0400 Subject: [PATCH] Navigate the active tab in place instead of spawning a new tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of "settings opens new tab for every click": every internal navigation went through `open_internal_tab → open_url → core.open_tab(url)`, and `open_tab` unconditionally inserts a new `BrowserTab`. So three settings sub-page clicks left four tabs in the sidebar, which is the screenshot the user keeps sending. Real browsers navigate the active tab in place for in-app links and spawn new tabs only on `+ New Tab` (or Cmd-click). Wire it through: * `BrowserTab::set_url(url)` mutates the tab's URL and bumps `last_active_at`. Title stays put — the page renderer can refresh it from the new URL. * `BrowserCore::navigate_active_tab(url)` finds the active tab, calls `set_url`, marks it Ready, records the history entry, and bumps activity. Returns `TabNotFound` if there's no active tab. * `ElyShell::navigate_active_tab` calls the core method and falls back to `open_tab` if there's no active tab to navigate. The shell's `open_internal_tab` (used by settings nav, home pills, sidebar Settings + Profile rows, command-overlay routes, etc.) now routes through this in-place path. * `open_url` keeps the explicit "spawn a new tab" semantics for `+ New Tab` and the deep-link router. Settings, plugin marketplace, history, profile picker — every sidebar nav now stays in one tab. cargo test --workspace: 440 passed, 0 failed. --- crates/ely_app/src/shell/navigation.rs | 30 ++++++++++++++++++++++- crates/ely_browser_core/src/state/tabs.rs | 22 +++++++++++++++++ crates/ely_domain/src/tab.rs | 10 ++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/crates/ely_app/src/shell/navigation.rs b/crates/ely_app/src/shell/navigation.rs index 72cf538..8312e0e 100644 --- a/crates/ely_app/src/shell/navigation.rs +++ b/crates/ely_app/src/shell/navigation.rs @@ -38,6 +38,12 @@ impl ElyShell { self.open_internal_tab("ely://task-manager", window, cx); } + /// In-tab navigation. Used by every settings sidebar item, every + /// home pill, and every "go to internal page" affordance — the + /// active tab follows the link instead of spawning a fresh tab + /// for every URL change. Use `open_url` only when the explicit + /// intent is "spawn a new tab" (see `open_new_tab` for the + /// keyboard shortcut path). pub(super) fn open_internal_tab( &mut self, url_text: &str, @@ -45,10 +51,32 @@ impl ElyShell { cx: &mut Context, ) { if let Ok(url) = UrlText::parse(url_text) { - self.open_url(url, window, cx); + self.navigate_active_tab(url, window, cx); } } + /// Navigate the active tab to `url` without creating a new tab. + /// Falls back to opening a new tab only if there's no active tab + /// to navigate (the BrowserCore returns `TabNotFound`). + pub(crate) fn navigate_active_tab( + &mut self, + url: UrlText, + window: &mut Window, + cx: &mut Context, + ) { + if let ShellState::Ready(core) = &mut self.state { + if core.navigate_active_tab(url.clone()).is_err() { + core.open_tab(url); + } + self.sync_address_input(window, cx); + self.focus_address_bar(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. pub(crate) fn open_url(&mut self, url: UrlText, window: &mut Window, cx: &mut Context) { if let ShellState::Ready(core) = &mut self.state { core.open_tab(url); diff --git a/crates/ely_browser_core/src/state/tabs.rs b/crates/ely_browser_core/src/state/tabs.rs index 62cb081..1bf4928 100644 --- a/crates/ely_browser_core/src/state/tabs.rs +++ b/crates/ely_browser_core/src/state/tabs.rs @@ -16,6 +16,28 @@ impl BrowserCore { Ok(self.open_tab(url)) } + /// Navigate the active tab to `url` in place. Used for clicks on + /// settings sub-pages, the home anchor, the bottom Settings row — + /// places where the user expects the current tab to follow the + /// link instead of accumulating a new tab for every step. + /// + /// Records a fresh history entry and resets activity timestamp; + /// no new tab is created and the active tab id is unchanged. + pub fn navigate_active_tab(&mut self, url: UrlText) -> Result<(), CoreError> { + let active_id = self.active_tab_id.clone(); + let tab_index = self + .tabs + .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].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(()) + } + pub fn open_tab(&mut self, url: UrlText) -> TabId { let tab = self.build_tab(url); let tab_id = tab.id().clone(); diff --git a/crates/ely_domain/src/tab.rs b/crates/ely_domain/src/tab.rs index d521a36..3da5bfe 100644 --- a/crates/ely_domain/src/tab.rs +++ b/crates/ely_domain/src/tab.rs @@ -270,6 +270,16 @@ impl BrowserTab { pub fn set_sync_enabled(&mut self, sync_enabled: bool) { 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. + pub fn set_url(&mut self, url: UrlText) { + self.url = url; + self.last_active_at = SystemTime::now(); + } } pub fn validate_zoom_percent(value: u16) -> Result {