diff --git a/CLAUDE.md b/CLAUDE.md index b6732eb..f1371b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ lembas info # paths + counts, useful when confused lembas secret-key # generate LEMBAS_SECRET_KEY lembas create-admin # create or promote an admin -pytest # 1394 tests, ~85s +pytest # 1399 tests, ~87s # PLAN.md tracks what is and is not built ruff check . # lint (line length 100) python scripts/build_artwork.py # regenerate artwork (SVG + PWA icons; @@ -838,11 +838,31 @@ folder of contracts into the window would cost the context on every request forever to answer one question. It therefore needs an existing chat, so it is absent on the new-chat screen — the same reason project files are. +**A page that uses `.page` needs `.admin-scroll` around it.** `.main` is a flex +column with `min-height: 0`, so content dropped straight into it overflows the +viewport with nothing to scroll — Save ends up below the bottom of the window, +reachable only by zooming out. `settings.html` gets this from `.tabs__body` and +the admin pages from `.admin-scroll`; the folder settings page shipped without +either. The two class names are one rule in `admin.css` for that reason. + +**A path is chosen, not typed.** `[data-dir-field]` in `ui.js` is the directory +picker on a form that is not the composer, scoped to that attribute so it and +the composer's own handler cannot both answer one click and open two dialogs. +The composer keeps its own because it does more: it follows the selected +profile's default directory until somebody picks their own, which only means +something while a chat is being created. + **The sidebar shows one kind at a time.** `Chat.kind` distinguishes an agent chat everywhere except the one place a person looked. The switch is stored on the account, and three things about it are not the obvious version. It lives *inside* the fragment it swaps, or the two buttons would go on showing the side -you had just left. `Folder.shown_in` hides a folder the filter emptied and keeps +you had just left — and "New chat", which sits *above* the scroll area rather +than in the tree, comes along out of band +(`partials/_sidebar_actions.html`, rendered with `oob` only by the fragment +route). That one shipped broken: the button went on saying "New chat" over a +list of agent chats. Whether it *worked* was never the question — it said one +thing and did another, which is the shape of failure the switch itself was +arranged to avoid. `Folder.shown_in` hides a folder the filter emptied and keeps one that was empty to begin with — the second is a container somebody just made, and hiding it means it can never be found again, let alone filed into. And with agent chats switched off there is no switch and no filtering at all, rather than 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") %} +
+ {% if can.get("chat.create") %} + {# Carries the side the switch is on, so this opens the new-chat screen + already on the right fork. #} + + {{ icon("plus", "icon--sm") }} + {{ "New agent chat" if sidebar_kind == "agent" else "New chat" }} + + {% endif %} + {% if can.get("folder.manage") %} + {# Asks for the name rather than making "New folder" and leaving somebody to + find the rename. `data-prompt` writes the answer into hx-vals before the + request goes out; see ui.js for why this is not htmx's own hx-prompt. #} + + {% endif %} +
+{% 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 %}
Folders
diff --git a/src/lembas/web/templates/partials/sidebar.html b/src/lembas/web/templates/partials/sidebar.html index ac3805e..3de2712 100644 --- a/src/lembas/web/templates/partials/sidebar.html +++ b/src/lembas/web/templates/partials/sidebar.html @@ -11,31 +11,7 @@ {{ brand(uid="side") }}
- {% if can.get("chat.create") or can.get("folder.manage") %} -
- {% if can.get("chat.create") %} - {# Carries the side the switch is on, so "New chat" on the Agent side opens - the new-chat screen already set to an agent chat. #} - - {{ icon("plus", "icon--sm") }} - {{ "New agent chat" if sidebar_kind == "agent" else "New chat" }} - - {% endif %} - {% if can.get("folder.manage") %} - {# Asks for the name rather than making "New folder" and leaving somebody to - find the rename. `data-prompt` writes the answer into hx-vals before the - request goes out; see ui.js for why this is not htmx's own hx-prompt. #} - - {% endif %} -
- {% 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)