Add tree base navigation UI

 - minimal functional implementation, not pretty
 - uses <details>, so no js needed
 - can switch at run time to test, but defaults to new UI

In order for the template to be able to know if the current item in the
tree is, or is a parent of, the currently selected path, I added the
ability to designate a currently selected path to PathItem, as we can't
express that logic cleanly in the DTL.

Note: supporting both UIs behind a feature flag intended to be short
lived, so that code should be deleted soon.

Fixes #94

Author: bloodearnest

airlock/file_browser_api.py

@@ -21,6 +21,11 @@ class PathItem:
     container: AirlockContainer
     relpath: pathlib.Path
 
+    # The currently selected view path in this container.  Used to calculate if
+    # how relpath relates to it, i.e. if relpath *is* the currently selected
+    # path, or is one of its parents.
+    selected: pathlib.Path = None
+
     def __post_init__(self):
         # ensure relpath is a Path
         object.__setattr__(self, "relpath", pathlib.Path(self.relpath))
@@ -45,14 +50,19 @@ def url(self):
 
     def parent(self):
         if self.relpath.parents:
-            return PathItem(self.container, self.relpath.parent)
+            return PathItem(self.container, self.relpath.parent, selected=self.selected)
 
     def children(self):
+        if not self.is_directory():
+            return []
         root = self.container.root()
-        return [
-            PathItem(self.container, child.relative_to(root))
+        children = [
+            PathItem(self.container, child.relative_to(root), selected=self.selected)
             for child in self._absolute_path().iterdir()
         ]
+        # directories first, then alphabetical, aks what windows does
+        children.sort(key=lambda p: (not p.is_directory(), p.relpath.name))
+        return children
 
     def siblings(self):
         if not self.relpath.parents:
@@ -63,10 +73,12 @@ def siblings(self):
     def contents(self):
         return self._absolute_path().read_text()
 
-    @property
     def suffix(self):
         return self.relpath.suffix
 
+    def file_type(self):
+        return self.suffix().lstrip(".")
+
     def breadcrumbs(self):
         item = self
         crumbs = [item]
@@ -79,3 +91,32 @@ def breadcrumbs(self):
 
         crumbs.reverse()
         return crumbs
+
+    def html_classes(self):
+        """Semantic html classes for this PathItem.
+
+        Currently, only "selected" is used, but it made sense to be able to
+        distinguish file/dirs, and maybe even file types, in the UI, in case we
+        need to.
+        """
+        classes = []
+
+        if self.is_directory():
+            classes.append("dir")
+        else:
+            classes.append("file")
+            classes.append(self.file_type())
+
+        if self.is_selected():
+            classes.append("selected")
+
+        return " ".join(classes)
+
+    def is_selected(self):
+        return self.relpath == self.selected
+
+    def is_on_selected_path(self):
+        return self.relpath in self.selected.parents
+
+    def is_open(self):
+        return self.is_selected() or self.is_on_selected_path()

airlock/settings.py

@@ -244,3 +244,6 @@ def run_connection_init_queries(*, connection, **kwargs):
     messages.WARNING: "alert-warning",
     messages.ERROR: "alert-danger",
 }
+
+# Temporary UI featureflag
+TREE = True

airlock/templates/file_browser/index.html

@@ -2,6 +2,51 @@
 
 {% load django_vite %}
 
+{% block extra_styles %}
+<style>
+
+/* tree styles */
+ul.tree {
+  list-style: none;
+
+  details ul {
+    border-left: 1px dotted grey;
+    padding-left: 0.25rem;
+    margin-left: 0.25rem;
+  }
+}
+
+
+.tree summary {
+  cursor: pointer;
+}
+
+.tree .selected {
+    font-weight: bold;
+    color: green;
+    background: rgba(0, 255, 0, 0.1);
+    cursor: pointer;
+}
+
+
+/* ths attempt at a nice selectedhighlight doesn't quite work
+.tree li {
+  position: relative;
+}
+
+&::before {
+  background: rgba(0, 255, 0, 0.1);
+  content: "";
+  position: absolute;
+  left: -100vw;
+  height: 100%;
+  width: 100vw;
+}
+*/
+
+</style>
+{% endblock extra_styles %}
+
 {% block full_width_content %}
 
 {% fragment as action_button %}
@@ -50,25 +95,32 @@
   <div style="flex-basis: 25%">
 
     {% #card %}
-      {% #list_group %}
-        {% if not path_item.parent %}
-          {% list_group_empty icon=True title=context|title|add:" Root" %}   
-        {% else %}
-          {% #list_group_item href=path_item.parent.url %}
-            ↰ ..
-          {% /list_group_item %}
-          {% for entry in path_item.siblings %}
-            {% #list_group_item href=entry.url %}
-              {{ entry.name}}
-              {% if entry.is_directory %}
-                {% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %}
-              {% endif %}
+      {% if tree %}
+        <ul class="tree root">
+        {% include "file_browser/tree.html" with path=root %}
+        </ul>
+      {% else %}
+        {% #list_group %}
+          {% if not path_item.parent %}
+            {% list_group_empty icon=True title=context|title|add:" Root" %}   
+          {% else %}
+            {% #list_group_item href=path_item.parent.url %}
+                ↰ ..
             {% /list_group_item %}
-          {% endfor %}
-        {% endif %}
-      {% /list_group %}
+            {% for entry in path_item.siblings %}
+                {% #list_group_item href=entry.url %}
+                {{ entry.name}}
+                {% if entry.is_directory %}
+                    {% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %}
+                {% endif %}
+                {% /list_group_item %}
+            {% endfor %}
+          {% endif %}
+        {% /list_group %}
+      {% endif %}
+ 
 
-    {% #card_footer no_container=False %}
+      {% #card_footer no_container=False %}
       {% if context == "request" %}
         <div class="px-4">
           {% #button variant="primary-outline" type="button" data-modal="viewFileGroups" %}
@@ -92,7 +144,6 @@
         {% /modal %}
         {% endif %}
       {% /card_footer %}
-
   {% /card %}
 
   </div>

airlock/templates/file_browser/tree.html

@@ -0,0 +1,18 @@
+{% for child in path.children %}
+<li class="tree ">
+  {% if child.is_directory %}
+  <details {% if child.is_open %}open{% endif %}>
+    <summary>
+      <a class="{{ child.html_classes }}" href="{{ child.url }}">{{ child.name }}</a>
+    </summary>
+    {% if child.children %}
+      <ul class="tree">
+        {% include "file_browser/tree.html" with path=child %}
+      </ul>
+    {% endif %}
+  </details>
+  {% else %}
+  <a class="{{ child.html_classes }}" {% if not child.is_selected %}href="{{ child.url }}"{% endif %}>{{ child.name }}</a>
+  {% endif %}
+</li>
+{% endfor %}

airlock/views.py

@@ -118,10 +118,23 @@ def workspace_index(request):
     return TemplateResponse(request, "workspaces.html", {"workspaces": workspaces})
 
 
+def use_tree_ui(request):
+    """Quick hack to be able to dynamically switch ui options."""
+    tree = request.session.get("tree", settings.TREE)
+    # hack to switch UI dynamically
+    if "tree" in request.GET:  # pragma: nocover
+        tree = request.GET["tree"].lower() == "true"
+
+    request.session["tree"] = tree
+    return tree
+
+
 def workspace_view(request, workspace_name: str, path: str = ""):
     workspace = validate_workspace(request.user, workspace_name)
 
-    path_item = PathItem(workspace, path)
+    relpath = Path(path)
+    root = PathItem(workspace, Path("."), selected=relpath)
+    path_item = PathItem(workspace, path, selected=relpath)
 
     if not path_item.exists():
         raise Http404()
@@ -135,6 +148,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
         "file_browser/index.html",
         {
             "workspace": workspace,
+            "root": root,
             "path_item": path_item,
             "context": "workspace",
             "title": f"Files for workspace {workspace_name}",
@@ -145,6 +159,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
             "form": AddFileForm(
                 release_request=api.get_current_request(workspace_name, request.user)
             ),
+            "tree": use_tree_ui(request),
         },
     )
 
@@ -200,7 +215,9 @@ def request_index(request):
 def request_view(request, request_id: str, path: str = ""):
     release_request = validate_release_request(request.user, request_id)
 
-    path_item = PathItem(release_request, path)
+    relpath = Path(path)
+    root = PathItem(release_request, Path("."), selected=relpath)
+    path_item = PathItem(release_request, path, selected=relpath)
 
     if not path_item.exists():
         raise Http404()
@@ -230,6 +247,7 @@ def request_view(request, request_id: str, path: str = ""):
     context = {
         "workspace": api.get_workspace(release_request.workspace),
         "release_request": release_request,
+        "root": root,
         "path_item": path_item,
         "context": "request",
         "title": f"Request for {release_request.workspace} by {release_request.author}",
@@ -239,6 +257,7 @@ def request_view(request, request_id: str, path: str = ""):
         "request_submit_url": request_submit_url,
         "request_reject_url": request_reject_url,
         "release_files_url": release_files_url,
+        "tree": use_tree_ui(request),
     }
 
     return TemplateResponse(request, "file_browser/index.html", context)

tests/integration/test_views.py

@@ -34,7 +34,13 @@ def client_with_permission(client_with_user):
     yield client_with_user(output_checker)
 
 
-def test_workspace_view(client_with_permission):
+# temporary fixture to test both UI options until we decide on one.
+@pytest.fixture(params=[True, False])
+def ui_options(request, settings):
+    settings.TREE = request.param
+
+
+def test_workspace_view(client_with_permission, ui_options):
     factories.write_workspace_file("workspace", "file.txt")
 
     response = client_with_permission.get("/workspaces/view/workspace/")
@@ -46,13 +52,13 @@ def test_workspace_does_not_exist(client_with_permission):
     assert response.status_code == 404
 
 
-def test_workspace_view_with_directory(client_with_permission):
+def test_workspace_view_with_directory(client_with_permission, ui_options):
     factories.write_workspace_file("workspace", "some_dir/file.txt")
     response = client_with_permission.get("/workspaces/view/workspace/some_dir/")
     assert "file.txt" in response.rendered_content
 
 
-def test_workspace_view_with_file(client_with_permission):
+def test_workspace_view_with_file(client_with_permission, ui_options):
     factories.write_workspace_file("workspace", "file.txt", "foobar")
     response = client_with_permission.get("/workspaces/view/workspace/file.txt")
     assert "foobar" in response.rendered_content
@@ -269,7 +275,7 @@ def test_request_index_no_user(client):
     assert response.status_code == 302
 
 
-def test_request_view_index(client_with_permission):
+def test_request_view_index(client_with_permission, ui_options):
     release_request = factories.create_release_request(
         "workspace", client_with_permission.user
     )
@@ -288,7 +294,7 @@ def test_request_id_does_not_exist(client_with_permission):
     assert response.status_code == 404
 
 
-def test_request_view_with_directory(client_with_permission):
+def test_request_view_with_directory(client_with_permission, ui_options):
     release_request = factories.create_release_request("workspace")
     factories.write_request_file(release_request, "some_dir/file.txt")
     response = client_with_permission.get(
@@ -297,7 +303,7 @@ def test_request_view_with_directory(client_with_permission):
     assert "file.txt" in response.rendered_content
 
 
-def test_request_view_with_file(client_with_permission):
+def test_request_view_with_file(client_with_permission, ui_options):
     release_request = factories.create_release_request("workspace")
     factories.write_request_file(release_request, "file.txt", "foobar")
     response = client_with_permission.get(

tests/unit/test_file_browser_api.py

@@ -103,6 +103,10 @@ def test_parent(container, path, parent_path):
             "some_dir",
             ["some_dir/file_a.txt", "some_dir/file_b.txt"],
         ),
+        (
+            "some_dir/file_a.txt",
+            [],
+        ),
     ],
 )
 def test_children(container, path, child_paths):
@@ -151,3 +155,21 @@ def test_breadcrumbs(container):
         PathItem(container, "foo/bar"),
         PathItem(container, "foo/bar/baz"),
     ]
+
+
+def test_selection_logic(container):
+    selected = Path("some_dir/file_a.txt")
+
+    selected_item = PathItem(container, selected, selected)
+    assert selected_item.is_selected()
+    assert selected_item.is_open()
+
+    parent_item = PathItem(container, "some_dir", selected)
+    assert not parent_item.is_selected()
+    assert parent_item.is_on_selected_path()
+    assert parent_item.is_open()
+
+    other_item = PathItem(container, "other_dir", selected)
+    assert not other_item.is_selected()
+    assert not other_item.is_on_selected_path()
+    assert not other_item.is_open()