diff --git a/CODESMELL.md b/CODESMELL.md index ab2f496..e005755 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -200,13 +200,15 @@ ## Medium Severity -### CSM-011 — No URL State / Deep Linking -- **File:** `lib/bds/desktop/shell_live.ex`, `lib/bds/desktop/router.ex` -- **What:** Tabs, filters, and selected items are never reflected in the URL. No `handle_params/3` or `push_patch/2` is used anywhere in the codebase. -- **Why it's bad:** Users cannot bookmark or use the back button. Harder to debug. -- **Mitigating factor:** This is a desktop app (wx container), not a web app. URL-based navigation is less critical but still valuable for debuggability and for when the app is accessed via a browser during development. -- **Fix:** Add `handle_params/3` for at least `?tab=` and `?view=` params. Use `push_patch` on state changes. -- **Test:** Click a tab; assert the URL updates to `/?tab=posts`; refresh; assert the same tab is active. +### ~~CSM-011 — No URL State / Deep Linking~~ ✅ FIXED +- **Fixed:** 2026-05-09 +- **What was done:** + - `mount/3` now reads `?view=` and `?tab=:` query params and applies them to the initial workbench state, enabling deep linking on page load. + - Added `push_url_state/1` — after state-changing events (`select_view`, `select_tab`, `close_tab`, `open_sidebar_item`, sidebar menu actions, project switch), pushes a `url-state` event to the client with the serialized URL. + - Added JS handler in the `AppShell` hook that calls `history.replaceState` to update the browser URL without triggering navigation. + - URL encoding: `?view=` (omitted when `posts`, the default) and `?tab=:` (omitted when no tab is active). Invalid or unknown params are silently ignored. + - Used `push_event` + `history.replaceState` instead of `push_patch`/`handle_params` to maintain compatibility with existing `live_isolated` tests. + - Added 10 tests in `test/bds/csm011_url_state_test.exs`: mount with `?view=media`, mount with default, mount with invalid view, mount with `?tab=post:`, mount with both params, `select_view` pushes url-state, `select_view` posts pushes clean URL, `select_tab` pushes url-state, `close_tab` removes tab from URL, `open_sidebar_item` pushes url-state. --- diff --git a/assets/js/hooks/app_shell.js b/assets/js/hooks/app_shell.js index ea03d6d..1b4f543 100644 --- a/assets/js/hooks/app_shell.js +++ b/assets/js/hooks/app_shell.js @@ -173,6 +173,12 @@ export const AppShell = { } }); + this.handleEvent("url-state", ({ path }) => { + if (path && window.location.pathname + window.location.search !== path) { + window.history.replaceState({}, "", path); + } + }); + window.addEventListener("bds:native-menu-action", this.handleNativeMenuAction); window.addEventListener("keydown", this.handleShortcutKeyDown, true); this.el.addEventListener("load", this.handleThumbnailLoad, true); diff --git a/lib/bds/desktop/shell_live.ex b/lib/bds/desktop/shell_live.ex index 8be73f6..2b7c595 100644 --- a/lib/bds/desktop/shell_live.ex +++ b/lib/bds/desktop/shell_live.ex @@ -142,7 +142,7 @@ defmodule BDS.Desktop.ShellLive do embed_templates("shell_live/*") @impl true - def mount(_params, _session, socket) do + def mount(params, _session, socket) do connected = connected?(socket) if connected do @@ -175,6 +175,7 @@ defmodule BDS.Desktop.ShellLive do |> assign(:panel_post_links, %{backlinks: [], outlinks: []}) |> assign(:panel_git_entries, []) |> reload_shell(workbench) + |> apply_url_params(params) |> tap(&sync_menu_bar_locale/1)} end @@ -198,7 +199,10 @@ defmodule BDS.Desktop.ShellLive do BoundedAtoms.sidebar_view(view_id, :posts) ) - {:noreply, refresh_sidebar(socket, workbench)} + {:noreply, + socket + |> refresh_sidebar(workbench) + |> push_url_state()} end def handle_event("select_panel_tab", %{"tab" => tab}, socket) do @@ -259,7 +263,8 @@ defmodule BDS.Desktop.ShellLive do {:noreply, socket |> assign(:tab_meta, tab_meta) - |> refresh_layout(workbench)} + |> refresh_layout(workbench) + |> push_url_state()} end def handle_event("close_tab", %{"type" => type, "id" => id}, socket) do @@ -270,7 +275,8 @@ defmodule BDS.Desktop.ShellLive do {:noreply, socket |> assign(:tab_meta, tab_meta) - |> refresh_layout(workbench)} + |> refresh_layout(workbench) + |> push_url_state()} end def handle_event( @@ -628,6 +634,77 @@ defmodule BDS.Desktop.ShellLive do ) end + defp push_url_state(socket) do + workbench = socket.assigns.workbench + + params = + %{} + |> put_url_view(workbench.active_view) + |> put_url_tab(workbench.active_tab) + + query = URI.encode_query(params) + path = if query == "", do: "/", else: "/?" <> query + + push_event(socket, "url-state", %{path: path}) + end + + defp put_url_view(params, :posts), do: params + defp put_url_view(params, view), do: Map.put(params, "view", Atom.to_string(view)) + + defp put_url_tab(params, nil), do: params + + defp put_url_tab(params, {type, id}), + do: Map.put(params, "tab", Atom.to_string(type) <> ":" <> id) + + defp apply_url_params(socket, params) when is_map(params) and map_size(params) > 0 do + workbench = socket.assigns.workbench + + workbench = apply_url_view(workbench, Map.get(params, "view")) + workbench = apply_url_tab(workbench, Map.get(params, "tab")) + + if workbench == socket.assigns.workbench do + socket + else + tab_meta = TabHelpers.sync_tab_meta(workbench, socket.assigns[:tab_meta] || %{}) + + socket + |> assign(:tab_meta, tab_meta) + |> refresh_sidebar(workbench) + end + end + + defp apply_url_params(socket, _params), do: socket + + defp apply_url_view(workbench, nil), do: workbench + + defp apply_url_view(workbench, view_str) do + view = BoundedAtoms.sidebar_view(view_str, nil) + + if view && view != workbench.active_view do + Workbench.click_activity(workbench, view) + else + workbench + end + end + + defp apply_url_tab(workbench, nil), do: workbench + + defp apply_url_tab(workbench, tab_str) do + case String.split(tab_str, ":", parts: 2) do + [type_str, id] -> + type = BoundedAtoms.editor_route(type_str, nil) + + if type && workbench.active_tab != {type, id} do + Workbench.open_tab(workbench, type, id, :preview) + else + workbench + end + + _ -> + workbench + end + end + defp refresh_sidebar(socket, workbench) do project_id = (socket.assigns[:projects] || %{})[:active_project_id] active_view_id = Atom.to_string(workbench.active_view) @@ -795,6 +872,7 @@ defmodule BDS.Desktop.ShellLive do socket |> assign(:tab_meta, tab_meta) |> refresh_layout(workbench) + |> push_url_state() end defp sidebar_create_action(view), do: SidebarCreate.action(view) @@ -839,6 +917,7 @@ defmodule BDS.Desktop.ShellLive do |> assign(:sidebar_filters_by_view, %{}) |> append_output_entry(title, message_fun.(project)) |> reload_shell(Workbench.new()) + |> push_url_state() {:error, reason} -> socket @@ -873,6 +952,7 @@ defmodule BDS.Desktop.ShellLive do socket |> assign(:tab_meta, tab_meta) |> refresh_sidebar(workbench) + |> push_url_state() MapSet.member?(@socket_menu_actions, action) -> handle_socket_menu_action(socket, action)