Repository: livebook-dev/kino (elixir)

Issue:
Expand tuples in Kino.Tree by default
Hi @stefanchrobot, what do you think about expanding Kino.Tree for tuples by default? Or for tuples with 6 or fewer elements? We often have 2 or 3 element tuples, and having to expand them manually makes navigation harder. :)

Pull request description:
`Kino.Tree`: auto-expand tuples of up to 6 elements
Closes #387.

Auto-expand tuples of up to 6 elements in `Kino.Tree` by default.

I've introduced the following changes:
- Add `"kind"` (e.g. `"tuple"`, `"list"`, etc.) attribute to tree's data,
- Use the `"kind"` attribute to change the logic for auto-expanding data; this now makes it easy to apply the same logic to lists and maps (if desired),
- Flatten the structure of the tree data: introduce `"expanded_before"` and `"expanded_after"` attributes,
- Refactor the UI code: rename `expanded` and `setExpanded` to `isExpanded` and `setIsExpanded` to better fit JavaScript conventions,
- Refactor the backend code: introduce `leaf_node` and `branch_node` to ensure that we build consistent data (without introducing more structs/types).


<img width="930" alt="Screenshot 2024-02-18 at 11 49 22" src="https://github.com/livebook-dev/kino/assets/8614029/5cedec08-c5d4-4cb6-afdc-5c179ab9aad1">

Patch:
diff --git a/assets/tree/src/App.js b/assets/tree/src/App.js
index 9ffc54e..20b999e 100644
--- a/assets/tree/src/App.js
+++ b/assets/tree/src/App.js
@@ -2,6 +2,15 @@ import React, { useState } from "react";
 import { RiArrowDownSFill, RiArrowRightSFill } from "@remixicon/react";
 import classNames from "classnames";
 
+const MAX_AUTO_EXPAND_SIZE = 6;
+
+function shouldAutoExpand(node, level) {
+  return (
+    level === 1 ||
+    (node.kind === "tuple" && node.children?.length <= MAX_AUTO_EXPAND_SIZE)
+  );
+}
+
 export default function App({ tree }) {
   return (
     <div className="font-mono text-sm text-gray-500">
@@ -11,11 +20,11 @@ export default function App({ tree }) {
 }
 
 function TreeNode({ node, level }) {
-  const [expanded, setExpanded] = useState(level === 1);
+  const [isExpanded, setIsExpanded] = useState(shouldAutoExpand(node, level));
 
   function handleExpandClick() {
     if (node.children) {
-      setExpanded(!expanded);
+      setIsExpanded(!isExpanded);
     }
   }
 
@@ -27,21 +36,21 @@ function TreeNode({ node, level }) {
       >
         <div className="mr-0.5 inline-block w-[2ch] flex-shrink-0">
           {node.children &&
-            (expanded ? (
+            (isExpanded ? (
               <RiArrowDownSFill size={20} />
             ) : (
               <RiArrowRightSFill size={20} />
             ))}
         </div>
         <div>
-          {node.children && expanded ? (
-            <TextItems items={node.expanded.prefix} />
+          {node.children && isExpanded ? (
+            <TextItems items={node.expanded_before} />
           ) : (
             <TextItems items={node.content} />
           )}
         </div>
       </div>
-      {node.children && expanded && (
+      {node.children && isExpanded && (
         <>
           <ol className="m-0 ml-[2ch] block list-none p-0">
             {node.children.map((child, index) => (
@@ -51,7 +60,7 @@ function TreeNode({ node, level }) {
             ))}
           </ol>
           <div className="ml-[2ch]">
-            <TextItems items={node.expanded.suffix} />
+            <TextItems items={node.expanded_after} />
           </div>
         </>
       )}
diff --git a/lib/kino/tree.ex b/lib/kino/tree.ex
index 40de8d5..3f289c2 100644
--- a/lib/kino/tree.ex
+++ b/lib/kino/tree.ex
@@ -43,7 +43,7 @@ defmodule Kino.Tree do
   end
 
   defp to_node(string, suffix) when is_binary(string) do
-    %{content: [green(inspect(string)) | suffix], children: nil}
+    leaf_node("binary", [green(inspect(string)) | suffix])
   end
 
   defp to_node(atom, suffix) when is_atom(atom) do
@@ -54,30 +54,25 @@ defmodule Kino.Tree do
         blue(inspect(atom))
       end
 
-    %{content: [span | suffix], children: nil}
+    leaf_node("atom", [span | suffix])
   end
 
   defp to_node(number, suffix) when is_number(number) do
-    %{content: [blue(inspect(number)) | suffix], children: nil}
+    leaf_node("number", [blue(inspect(number)) | suffix])
   end
 
   defp to_node({}, suffix) do
-    %{content: [black("{}") | suffix], children: nil}
+    leaf_node("tuple", [black("{}") | suffix])
   end
 
   defp to_node(tuple, suffix) when is_tuple(tuple) do
     size = tuple_size(tuple)
     children = tuple |> Tuple.to_list() |> to_children(size)
-
-    %{
-      content: [black("{...}") | suffix],
-      children: children,
-      expanded: %{prefix: [black("{")], suffix: [black("}") | suffix]}
-    }
+    branch_node("tuple", [black("{...}") | suffix], children, [black("{")], [black("}") | suffix])
   end
 
   defp to_node([], suffix) do
-    %{content: [black("[]") | suffix], children: nil}
+    leaf_node("list", [black("[]") | suffix])
   end
 
   defp to_node(list, suffix) when is_list(list) do
@@ -90,53 +85,43 @@ defmodule Kino.Tree do
         to_children(list, size)
       end
 
-    %{
-      content: [black("[...]") | suffix],
-      children: children,
-      expanded: %{prefix: [black("[")], suffix: [black("]") | suffix]}
-    }
+    branch_node("list", [black("[...]") | suffix], children, [black("[")], [black("]") | suffix])
   end
 
   defp to_node(%Regex{} = regex, suffix) do
-    %{content: [red(inspect(regex)) | suffix], children: nil}
+    leaf_node("regex", [red(inspect(regex)) | suffix])
   end
 
   defp to_node(%module{} = struct, suffix) when is_struct(struct) do
     if Inspect.impl_for(struct) != Inspect.Any do
-      %{content: [black(inspect(struct)) | suffix], children: nil}
+      leaf_node("struct", [black(inspect(struct)) | suffix])
     else
       map = Map.from_struct(struct)
       size = map_size(map)
       children = to_key_value_children(map, size)
 
-      %{
-        content: [black("%"), blue(inspect(module)), black("{...}") | suffix],
-        children: children,
-        expanded: %{
-          prefix: [black("%"), blue(inspect(module)), black("{")],
-          suffix: [black("}") | suffix]
-        }
-      }
+      branch_node(
+        "struct",
+        [black("%"), blue(inspect(module)), black("{...}") | suffix],
+        children,
+        [black("%"), blue(inspect(module)), black("{")],
+        [black("}") | suffix]
+      )
     end
   end
 
   defp to_node(%{} = map, suffix) when map_size(map) == 0 do
-    %{content: [black("%{}") | suffix], children: nil}
+    leaf_node("map", [black("%{}") | suffix])
   end
 
   defp to_node(map, suffix) when is_map(map) do
     size = map_size(map)
     children = map |> Enum.sort() |> to_key_value_children(size)
-
-    %{
-      content: [black("%{...}") | suffix],
-      children: children,
-      expanded: %{prefix: [black("%{")], suffix: [black("}") | suffix]}
-    }
+    branch_node("map", [black("%{...}") | suffix], children, [black("%{")], [black("}") | suffix])
   end
 
   defp to_node(other, suffix) do
-    %{content: [black(inspect(other)) | suffix], children: nil}
+    leaf_node("other", [black(inspect(other)) | suffix])
   end
 
   defp to_key_value_node({key, value}, suffix) do
@@ -150,15 +135,15 @@ defmodule Kino.Tree do
       end
 
     case to_node(value, suffix) do
-      %{content: content, expanded: %{prefix: prefix} = expanded} = node ->
+      %{content: content, children: nil} = node ->
+        %{node | content: [key_span, sep_span | content]}
+
+      %{content: content, expanded_before: expanded_before} = node ->
         %{
           node
           | content: [key_span, sep_span | content],
-            expanded: %{expanded | prefix: [key_span, sep_span | prefix]}
+            expanded_before: [key_span, sep_span | expanded_before]
         }
-
-      %{content: content} = node ->
-        %{node | content: [key_span, sep_span | content]}
     end
   end
 
@@ -182,6 +167,26 @@ defmodule Kino.Tree do
     end
   end
 
+  defp leaf_node(kind, content) do
+    %{
+      kind: kind,
+      content: content,
+      children: nil,
+      expanded_before: nil,
+      expanded_after: nil
+    }
+  end
+
+  defp branch_node(kind, content, children, expanded_before, expanded_after) do
+    %{
+      kind: kind,
+      content: content,
+      children: children,
+      expanded_before: expanded_before,
+      expanded_after: expanded_after
+    }
+  end
+
   defp black(text), do: %{text: text, color: nil}
   defp red(text), do: %{text: text, color: "var(--ansi-color-red)"}
   defp green(text), do: %{text: text, color: "var(--ansi-color-green)"}


Test patch:
diff --git a/test/kino/tree_test.exs b/test/kino/tree_test.exs
index 54cdc87..e5d7911 100644
--- a/test/kino/tree_test.exs
+++ b/test/kino/tree_test.exs
@@ -2,40 +2,47 @@ defmodule Kino.TreeTest do
   use Kino.LivebookCase, async: true
 
   test "renders strings as string nodes" do
-    assert %{content: ~s("some string"), children: nil} = plaintext_tree("some string")
+    assert %{kind: "binary", content: ~s("some string"), children: nil} =
+             plaintext_tree("some string")
   end
 
   test "renders atoms as nodes with inspected value" do
-    assert %{content: ":foo", children: nil} = plaintext_tree(:foo)
-    assert %{content: ~s(:"I need quotes"), children: nil} = plaintext_tree(:"I need quotes")
-    assert %{content: "SomeModule", children: nil} = plaintext_tree(SomeModule)
+    assert %{kind: "atom", content: ":foo", children: nil} = plaintext_tree(:foo)
+    assert %{kind: "atom", content: ~s(:"quote me"), children: nil} = plaintext_tree(:"quote me")
+    assert %{kind: "atom", content: "SomeModule", children: nil} = plaintext_tree(SomeModule)
   end
 
   test "renders numbers as nodes with inspected value" do
-    assert %{content: "100", children: nil} = plaintext_tree(100)
-    assert %{content: "100.0", children: nil} = plaintext_tree(100.0)
+    assert %{kind: "number", content: "100", children: nil} = plaintext_tree(100)
+    assert %{kind: "number", content: "100.0", children: nil} = plaintext_tree(100.0)
   end
 
   test "renders tuples as nodes with children" do
     assert %{
+             kind: "tuple",
              content: "{...}",
-             expanded: %{prefix: "{", suffix: "}"},
+             expanded_before: "{",
+             expanded_after: "}",
              children: [
-               %{content: "1", children: nil}
+               %{kind: "number", content: "1", children: nil}
              ]
            } = plaintext_tree({1})
   end
 
   test "handles deep nesting" do
     assert %{
+             kind: "tuple",
              content: "{...}",
-             expanded: %{prefix: "{", suffix: "}"},
+             expanded_before: "{",
+             expanded_after: "}",
              children: [
                %{
+                 kind: "tuple",
                  content: "{...}",
-                 expanded: %{prefix: "{", suffix: "}"},
+                 expanded_before: "{",
+                 expanded_after: "}",
                  children: [
-                   %{content: "1", children: nil}
+                   %{kind: "number", content: "1", children: nil}
                  ]
                }
              ]
@@ -44,80 +51,96 @@ defmodule Kino.TreeTest do
 
   test "adds trailing commas to all but last child" do
     assert %{
+             kind: "tuple",
              content: "{...}",
-             expanded: %{prefix: "{", suffix: "}"},
+             expanded_before: "{",
+             expanded_after: "}",
              children: [
-               %{content: "1,", children: nil},
+               %{kind: "number", content: "1,", children: nil},
                %{
+                 kind: "tuple",
                  content: "{...},",
-                 expanded: %{prefix: "{", suffix: "},"},
+                 expanded_before: "{",
+                 expanded_after: "},",
                  children: [
-                   %{content: ":x,", children: nil},
-                   %{content: ":y", children: nil}
+                   %{kind: "atom", content: ":x,", children: nil},
+                   %{kind: "atom", content: ":y", children: nil}
                  ]
                },
-               %{content: ":three", children: nil}
+               %{kind: "atom", content: ":three", children: nil}
              ]
            } = plaintext_tree({1, {:x, :y}, :three})
   end
 
   test "renders lists as nodes with children" do
     assert %{
+             kind: "list",
              content: "[...]",
-             expanded: %{prefix: "[", suffix: "]"},
+             expanded_before: "[",
+             expanded_after: "]",
              children: [
-               %{content: "1", children: nil}
+               %{kind: "number", content: "1", children: nil}
              ]
            } = plaintext_tree([1])
   end
 
   test "renders keywords as nodes with key-value children" do
     assert %{
+             kind: "list",
              content: "[...]",
-             expanded: %{prefix: "[", suffix: "]"},
+             expanded_before: "[",
+             expanded_after: "]",
              children: [
-               %{content: "foo: :bar", children: nil}
+               %{kind: "atom", content: "foo: :bar", children: nil}
              ]
            } = plaintext_tree(foo: :bar)
   end
 
   test "renders maps as nodes with key-value children" do
     assert %{
+             kind: "map",
              content: "%{...}",
-             expanded: %{prefix: "%{", suffix: "}"},
+             expanded_before: "%{",
+             expanded_after: "}",
              children: [
-               %{content: "foo: :bar", children: nil}
+               %{kind: "atom", content: "foo: :bar", children: nil}
              ]
            } = plaintext_tree(%{foo: :bar})
   end
 
   test "uses the arrow for non-atom keys" do
     assert %{
+             kind: "map",
              content: "%{...}",
-             expanded: %{prefix: "%{", suffix: "}"},
+             expanded_before: "%{",
+             expanded_after: "}",
              children: [
-               %{content: ~s("foo" => "bar"), children: nil}
+               %{kind: "binary", content: ~s("foo" => "bar"), children: nil}
              ]
            } = plaintext_tree(%{"foo" => "bar"})
   end
 
   test "sorts maps by key" do
     assert %{
+             kind: "map",
              content: "%{...}",
-             expanded: %{prefix: "%{", suffix: "}"},
+             expanded_before: "%{",
+             expanded_after: "}",
              children: [
-               %{content: "bar: :baz,", children: nil},
-               %{content: "foo: :oof", children: nil}
+               %{kind: "atom", content: "bar: :baz,", children: nil},
+               %{kind: "atom", content: "foo: :oof", children: nil}
              ]
            } = plaintext_tree(%{foo: :oof, bar: :baz})
   end
 
   test "uses Inspect protocol for compound keys" do
     assert %{
+             kind: "map",
              content: "%{...}",
-             expanded: %{prefix: "%{", suffix: "}"},
+             expanded_before: "%{",
+             expanded_after: "}",
              children: [
-               %{content: "{1, 2} => true", children: nil}
+               %{kind: "atom", content: "{1, 2} => true", children: nil}
              ]
            } = plaintext_tree(%{{1, 2} => true})
   end
@@ -128,25 +151,28 @@ defmodule Kino.TreeTest do
 
   test "renders structs as nodes with children" do
     assert %{
+             kind: "struct",
              content: "%Kino.TreeTest.User{...}",
-             expanded: %{prefix: "%Kino.TreeTest.User{", suffix: "}"},
+             expanded_before: "%Kino.TreeTest.User{",
+             expanded_after: "}",
              children: [
-               %{content: ~s(email: "user@example.com"), children: nil}
+               %{kind: "binary", content: ~s(email: "user@example.com"), children: nil}
              ]
            } = plaintext_tree(%User{email: "user@example.com"})
   end
 
   test "uses special handling for regexes" do
-    assert %{content: "~r/foobar/", children: nil} = plaintext_tree(~r/foobar/)
-    assert %{content: "~r//", children: nil} = plaintext_tree(%Regex{})
+    assert %{kind: "regex", content: "~r/foobar/", children: nil} = plaintext_tree(~r/foobar/)
+    assert %{kind: "regex", content: "~r//", children: nil} = plaintext_tree(%Regex{})
   end
 
   test "uses the Inspect protocol for structs that implement it" do
-    assert %{content: "~D[2022-01-01]", children: nil} = plaintext_tree(Date.new!(2022, 1, 1))
+    assert %{kind: "struct", content: "~D[2022-01-01]", children: nil} =
+             plaintext_tree(Date.new!(2022, 1, 1))
   end
 
   test "renders other terms as string nodes using Inspect protocol" do
-    assert %{content: "#PID<" <> _rest, children: nil} = plaintext_tree(self())
+    assert %{kind: "other", content: "#PID<" <> _rest, children: nil} = plaintext_tree(self())
   end
 
   test "renders empty containers as leaf nodes" do
@@ -169,7 +195,8 @@ defmodule Kino.TreeTest do
   test "uses separate colors for keys and values" do
     assert %{
              content: [%{text: "[...]", color: nil}],
-             expanded: %{prefix: [%{text: "[", color: nil}], suffix: [%{text: "]", color: nil}]},
+             expanded_before: [%{text: "[", color: nil}],
+             expanded_after: [%{text: "]", color: nil}],
              children: [
                %{
                  content: [
@@ -207,25 +234,27 @@ defmodule Kino.TreeTest do
     Enum.map(list, &plaintext/1)
   end
 
+  defp plaintext(%{content: content, children: nil} = node) do
+    %{node | content: text_of(content)}
+  end
+
   defp plaintext(
          %{
            content: content,
            children: children,
-           expanded: %{prefix: prefix, suffix: suffix} = expanded
+           expanded_before: expanded_before,
+           expanded_after: expanded_after
          } = node
        ) do
     %{
       node
       | content: text_of(content),
         children: Enum.map(children, &plaintext/1),
-        expanded: %{expanded | prefix: text_of(prefix), suffix: text_of(suffix)}
+        expanded_before: text_of(expanded_before),
+        expanded_after: text_of(expanded_after)
     }
   end
 
-  defp plaintext(%{content: content, children: nil} = node) do
-    %{node | content: text_of(content)}
-  end
-
   defp text_of(list) when is_list(list) do
     list |> Enum.map(& &1.text) |> Enum.join()
   end