Navigate the active tab in place instead of spawning a new tab
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.
This commit is contained in:
@@ -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<Self>,
|
||||
) {
|
||||
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<Self>,
|
||||
) {
|
||||
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<Self>) {
|
||||
if let ShellState::Ready(core) = &mut self.state {
|
||||
core.open_tab(url);
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<u16, DomainError> {
|
||||
|
||||
Reference in New Issue
Block a user