diff --git a/src/lembas/api/preferences.py b/src/lembas/api/preferences.py index 38f392f..24c9ab8 100644 --- a/src/lembas/api/preferences.py +++ b/src/lembas/api/preferences.py @@ -108,7 +108,10 @@ async def set_sidebar_kind( return templates.TemplateResponse( request, "partials/_sidebar_tree.html", - {"chat": None, "user": user, **sidebar_context(db, user)}, + # `oob` brings the New chat button along out of band. It sits above the + # scroll area rather than inside the tree, so a swap of the tree alone + # left it saying "New chat" while agent chats were listed underneath. + {"chat": None, "user": user, "oob": True, **sidebar_context(db, user)}, ) diff --git a/src/lembas/web/static/css/admin.css b/src/lembas/web/static/css/admin.css index 014fcec..44bf28f 100644 --- a/src/lembas/web/static/css/admin.css +++ b/src/lembas/web/static/css/admin.css @@ -17,6 +17,20 @@ padding: var(--sp-6) var(--sp-5) var(--sp-12); } +/* A directory chosen rather than typed. The path is the only part allowed to + shrink, and it truncates -- an absolute path on somebody else's machine is + long enough to push the Clear button off the card otherwise. */ +.dir-row { display: flex; align-items: center; gap: var(--sp-2); min-width: 0; } +.dir-row > .btn { min-width: 0; } +.dir-row__path { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-family: var(--font-mono); + font-size: var(--text-xs); +} + .page-header { margin-bottom: var(--sp-6); } .admin-lede, .page-header__lede { diff --git a/src/lembas/web/static/js/ui.js b/src/lembas/web/static/js/ui.js index 0dc410e..1c4f144 100644 --- a/src/lembas/web/static/js/ui.js +++ b/src/lembas/web/static/js/ui.js @@ -598,6 +598,50 @@ document.addEventListener("lembas:notify", function (event) { setDir(profileDefault()); } + /* The same directory picker, on a form that is not the composer. + + Scoped to `[data-dir-field]`, which the composer's own markup does not + carry -- otherwise this and `wire()` above would both answer one click and + open two dialogs. The composer keeps its own handler because it has more to + do: it follows the selected profile's default directory until somebody + chooses their own, which only makes sense while a chat is being created. + + A path is something you would rather find than spell, and the text box this + replaced was the one control on the folder page that asked somebody to + remember an absolute path on another machine. */ + document.addEventListener("click", function (event) { + var field = event.target.closest("[data-dir-field]"); + if (!field) return; + + var value = field.querySelector("[data-dir-value]"); + var label = field.querySelector("[data-dir-label]"); + if (!value) return; + + function show(path) { + value.value = path || ""; + if (label) label.textContent = path || "the connection's own default"; + } + + if (event.target.closest("[data-dir-clear]")) { + event.preventDefault(); + show(""); + return; + } + if (!event.target.closest("[data-dir-browse]")) return; + event.preventDefault(); + + /* The connection this folder points at. Browsing needs one, and saying so + beats a dialog that opens onto nothing. */ + var form = field.closest("form"); + var picker = form && form.querySelector('select[name="ssh_profile_id"]'); + var profileId = picker ? picker.value : ""; + if (!profileId) { + window.lembas.notify("Choose a connection first — there is nothing to browse without one."); + return; + } + window.lembas.chooseDirectory(profileId, value.value, show); + }); + function scan() { document.querySelectorAll("[data-agent-picker]").forEach(function (el) { if (!el.dataset.wired) { el.dataset.wired = "1"; wire(el); } diff --git a/src/lembas/web/templates/folders/edit.html b/src/lembas/web/templates/folders/edit.html index 1cf356d..d0cec19 100644 --- a/src/lembas/web/templates/folders/edit.html +++ b/src/lembas/web/templates/folders/edit.html @@ -39,6 +39,14 @@ + {# + `.admin-scroll` is what scrolls, not `.page`. `.main` is a flex column + with `min-height: 0`, so a `.page` dropped straight into it overflows the + viewport with nothing to scroll -- Save and Back end up somewhere below + the bottom of the window, reachable only by zooming out. Every other page + of this shape already wraps its content this way; this one did not. + #} +
@@ -128,11 +136,33 @@
-
- - + {# A button and a hidden field, not a text box -- the same shape the + new-chat screen uses, and for the same reason it gives there: a + path is something you would rather find than spell. `data-dir-field` + is what wires it; see ui.js. #} +
+ Project directory + +
+ + {# Always here, not only when something is stored: browsing to a + directory and then changing your mind before saving needs a way + back to "no opinion" as much as clearing a saved one does. #} + +
+

+ What a relative path is measured from in chats started here. +

{% if agent_modes %} @@ -158,6 +188,7 @@
+
{% endblock %} diff --git a/src/lembas/web/templates/partials/_sidebar_actions.html b/src/lembas/web/templates/partials/_sidebar_actions.html new file mode 100644 index 0000000..1efcc2b --- /dev/null +++ b/src/lembas/web/templates/partials/_sidebar_actions.html @@ -0,0 +1,40 @@ +{% from "_macros.html" import icon %} +{# + New chat, and New folder. + + Its own partial because "New chat" changes with the Chat/Agent switch and this + row sits *outside* the tree the switch swaps -- above the scroll area, so it + does not scroll away. It arrives out of band instead, the move the `done` + frame already makes for the chat title. + + It was not a partial to begin with, and the button went on saying "New chat" + after the switch moved to Agents while the tree beneath it showed agent chats. + Whether it worked was not the question: it said one thing and did another, + which is exactly the shape of failure the switch itself was arranged to avoid. +#} +{% if can.get("chat.create") or can.get("folder.manage") %} + +{% endif %} diff --git a/src/lembas/web/templates/partials/_sidebar_tree.html b/src/lembas/web/templates/partials/_sidebar_tree.html index 0fd6f9c..8da472f 100644 --- a/src/lembas/web/templates/partials/_sidebar_tree.html +++ b/src/lembas/web/templates/partials/_sidebar_tree.html @@ -46,6 +46,19 @@ {% endif %} + {# + "New chat" becomes "New agent chat" with the switch, and it lives above the + scroll area rather than inside this fragment -- so it comes along out of + band. Only when this is the fragment response: on a full page load the row + is already rendered in its own place, and a second copy marked + `hx-swap-oob` would be a stray element sitting in the tree. + #} + {% if oob %} + {% with oob = true %} + {% include "partials/_sidebar_actions.html" %} + {% endwith %} + {% endif %} + {% if folders %} - {% if can.get("chat.create") or can.get("folder.manage") %} - - {% endif %} + {% include "partials/_sidebar_actions.html" %} {# Every 10s, refresh the unread dots and announce anything that finished while this page was showing something else. Out-of-band spans only, so the diff --git a/tests/test_folders.py b/tests/test_folders.py index a465e54..2dd1334 100644 --- a/tests/test_folders.py +++ b/tests/test_folders.py @@ -290,6 +290,58 @@ def test_the_settings_page_renders_what_is_stored(client: TestClient, db, regist assert f'hx-patch="/api/folders/{folder.id}"' in page +def test_the_settings_page_can_be_scrolled(client: TestClient, db, registered): + """`.main` is a flex column with `min-height: 0`, so a `.page` dropped + straight into it overflows the viewport with nothing to scroll and Save ends + up below the bottom of the window. Every other page of this shape wraps its + content in the scrolling container; this one did not.""" + folder = _folder(db, "Errands") + page = client.get(f"/folders/{folder.id}").text + assert 'class="admin-scroll"' in page + assert page.index('class="admin-scroll"') < page.index('class="page"') + + +def test_the_project_directory_is_chosen_rather_than_typed( + client: TestClient, db, registered +): + """The same control the new-chat screen uses, and for the reason it gives + there: a path on another machine is something you would rather find than + spell.""" + _add_connection(db) + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + db.add( + SshProfile( + owner_id=db.scalars(select(User).order_by(User.created_at)).first().id, + name="Box", + host="example.test", + username="root", + host_key="ssh-ed25519 AAAA", + ) + ) + db.commit() + folder = _folder(db, "Errands", project_dir="/srv/project") + + page = client.get(f"/folders/{folder.id}").text + assert "data-dir-field" in page + assert "data-dir-browse" in page + # The hidden field is what the form actually submits, so it must carry the + # stored value -- the button beside it only shows it. + assert 'name="project_dir"' in page + assert 'value="/srv/project"' in page + assert 'type="hidden"' in page + + +def test_the_directory_can_still_be_cleared(client: TestClient, db, registered): + """A hidden input always submits, so an empty one means "no opinion" rather + than "leave it alone" -- which is the whole reason `update_folder` reads the + raw form.""" + _add_connection(db) + folder = _folder(db, "Errands", project_dir="/srv/project") + client.patch(f"/api/folders/{folder.id}", data={"project_dir": ""}) + db.refresh(folder) + assert folder.project_dir == "" + + def test_the_settings_page_refuses_someone_elses_folder(client: TestClient, db, registered): other = User(name="Sam", email="sam@shire.test", password_hash="x") db.add(other) diff --git a/tests/test_sidebar_split.py b/tests/test_sidebar_split.py index 67dac25..5742fc4 100644 --- a/tests/test_sidebar_split.py +++ b/tests/test_sidebar_split.py @@ -77,6 +77,43 @@ def test_an_unknown_kind_is_refused_rather_than_stored(client: TestClient, db, r assert "sidebar_kind" not in (user.settings_json or {}) +def test_the_new_chat_button_follows_the_switch(client: TestClient, db, registered): + """It sits above the scroll area rather than inside the tree, so a swap of + the tree alone left it saying "New chat" while agent chats were listed + underneath -- saying one thing and doing another, which is the shape of + failure the switch itself was arranged to avoid. + + Asserted on the *fragment*, not on a page load: a page load re-renders the + button anyway, which is exactly why this went unnoticed. + """ + _add_connection(db) + _enable_agents(db) + + response = client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + assert 'id="sidebar-actions"' in response.text + assert 'hx-swap-oob="true"' in response.text + assert "New agent chat" in response.text + assert 'href="/chat?kind=agent"' in response.text + + response = client.post("/api/preferences/sidebar-kind", data={"kind": "chat"}) + assert "New agent chat" not in response.text + assert 'href="/chat"' in response.text + + +def test_a_page_load_carries_no_stray_out_of_band_row( + client: TestClient, db, registered +): + """`hx-swap-oob` on a full page load would be a duplicate element sitting in + the tree, waiting to be swapped by the next unrelated request.""" + _add_connection(db) + _enable_agents(db) + + page = client.get("/chat").text + assert page.count('id="sidebar-actions"') == 1 + assert 'id="sidebar-actions"\n hx-swap-oob' not in page + assert "hx-swap-oob" not in page.split('id="sidebar-tree"')[1][:2000] + + def test_the_switch_survives_a_page_load(client: TestClient, db, registered): _add_connection(db) _enable_agents(db)