fix: fixed CSM-011
This commit is contained in:
16
CODESMELL.md
16
CODESMELL.md
@@ -200,13 +200,15 @@
|
|||||||
|
|
||||||
## Medium Severity
|
## Medium Severity
|
||||||
|
|
||||||
### CSM-011 — No URL State / Deep Linking
|
### ~~CSM-011 — No URL State / Deep Linking~~ ✅ FIXED
|
||||||
- **File:** `lib/bds/desktop/shell_live.ex`, `lib/bds/desktop/router.ex`
|
- **Fixed:** 2026-05-09
|
||||||
- **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.
|
- **What was done:**
|
||||||
- **Why it's bad:** Users cannot bookmark or use the back button. Harder to debug.
|
- `mount/3` now reads `?view=` and `?tab=<type>:<id>` query params and applies them to the initial workbench state, enabling deep linking on page load.
|
||||||
- **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.
|
- 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.
|
||||||
- **Fix:** Add `handle_params/3` for at least `?tab=` and `?view=` params. Use `push_patch` on state changes.
|
- Added JS handler in the `AppShell` hook that calls `history.replaceState` to update the browser URL without triggering navigation.
|
||||||
- **Test:** Click a tab; assert the URL updates to `/?tab=posts`; refresh; assert the same tab is active.
|
- URL encoding: `?view=<sidebar_view>` (omitted when `posts`, the default) and `?tab=<type>:<id>` (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:<id>`, 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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -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("bds:native-menu-action", this.handleNativeMenuAction);
|
||||||
window.addEventListener("keydown", this.handleShortcutKeyDown, true);
|
window.addEventListener("keydown", this.handleShortcutKeyDown, true);
|
||||||
this.el.addEventListener("load", this.handleThumbnailLoad, true);
|
this.el.addEventListener("load", this.handleThumbnailLoad, true);
|
||||||
|
|||||||
@@ -142,7 +142,7 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
embed_templates("shell_live/*")
|
embed_templates("shell_live/*")
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def mount(_params, _session, socket) do
|
def mount(params, _session, socket) do
|
||||||
connected = connected?(socket)
|
connected = connected?(socket)
|
||||||
|
|
||||||
if connected do
|
if connected do
|
||||||
@@ -175,6 +175,7 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
|> assign(:panel_post_links, %{backlinks: [], outlinks: []})
|
|> assign(:panel_post_links, %{backlinks: [], outlinks: []})
|
||||||
|> assign(:panel_git_entries, [])
|
|> assign(:panel_git_entries, [])
|
||||||
|> reload_shell(workbench)
|
|> reload_shell(workbench)
|
||||||
|
|> apply_url_params(params)
|
||||||
|> tap(&sync_menu_bar_locale/1)}
|
|> tap(&sync_menu_bar_locale/1)}
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -198,7 +199,10 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
BoundedAtoms.sidebar_view(view_id, :posts)
|
BoundedAtoms.sidebar_view(view_id, :posts)
|
||||||
)
|
)
|
||||||
|
|
||||||
{:noreply, refresh_sidebar(socket, workbench)}
|
{:noreply,
|
||||||
|
socket
|
||||||
|
|> refresh_sidebar(workbench)
|
||||||
|
|> push_url_state()}
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_event("select_panel_tab", %{"tab" => tab}, socket) do
|
def handle_event("select_panel_tab", %{"tab" => tab}, socket) do
|
||||||
@@ -259,7 +263,8 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
{:noreply,
|
{:noreply,
|
||||||
socket
|
socket
|
||||||
|> assign(:tab_meta, tab_meta)
|
|> assign(:tab_meta, tab_meta)
|
||||||
|> refresh_layout(workbench)}
|
|> refresh_layout(workbench)
|
||||||
|
|> push_url_state()}
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_event("close_tab", %{"type" => type, "id" => id}, socket) do
|
def handle_event("close_tab", %{"type" => type, "id" => id}, socket) do
|
||||||
@@ -270,7 +275,8 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
{:noreply,
|
{:noreply,
|
||||||
socket
|
socket
|
||||||
|> assign(:tab_meta, tab_meta)
|
|> assign(:tab_meta, tab_meta)
|
||||||
|> refresh_layout(workbench)}
|
|> refresh_layout(workbench)
|
||||||
|
|> push_url_state()}
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_event(
|
def handle_event(
|
||||||
@@ -628,6 +634,77 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
)
|
)
|
||||||
end
|
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
|
defp refresh_sidebar(socket, workbench) do
|
||||||
project_id = (socket.assigns[:projects] || %{})[:active_project_id]
|
project_id = (socket.assigns[:projects] || %{})[:active_project_id]
|
||||||
active_view_id = Atom.to_string(workbench.active_view)
|
active_view_id = Atom.to_string(workbench.active_view)
|
||||||
@@ -795,6 +872,7 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
socket
|
socket
|
||||||
|> assign(:tab_meta, tab_meta)
|
|> assign(:tab_meta, tab_meta)
|
||||||
|> refresh_layout(workbench)
|
|> refresh_layout(workbench)
|
||||||
|
|> push_url_state()
|
||||||
end
|
end
|
||||||
|
|
||||||
defp sidebar_create_action(view), do: SidebarCreate.action(view)
|
defp sidebar_create_action(view), do: SidebarCreate.action(view)
|
||||||
@@ -839,6 +917,7 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
|> assign(:sidebar_filters_by_view, %{})
|
|> assign(:sidebar_filters_by_view, %{})
|
||||||
|> append_output_entry(title, message_fun.(project))
|
|> append_output_entry(title, message_fun.(project))
|
||||||
|> reload_shell(Workbench.new())
|
|> reload_shell(Workbench.new())
|
||||||
|
|> push_url_state()
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
socket
|
socket
|
||||||
@@ -873,6 +952,7 @@ defmodule BDS.Desktop.ShellLive do
|
|||||||
socket
|
socket
|
||||||
|> assign(:tab_meta, tab_meta)
|
|> assign(:tab_meta, tab_meta)
|
||||||
|> refresh_sidebar(workbench)
|
|> refresh_sidebar(workbench)
|
||||||
|
|> push_url_state()
|
||||||
|
|
||||||
MapSet.member?(@socket_menu_actions, action) ->
|
MapSet.member?(@socket_menu_actions, action) ->
|
||||||
handle_socket_menu_action(socket, action)
|
handle_socket_menu_action(socket, action)
|
||||||
|
|||||||
Reference in New Issue
Block a user