Asset Browser: Rework layout & behavior of catalog tree-view

This reworks how tree rows are constructed in the layout and how they
behave in return.

* To open or collapse a row, the triangle/chevron icon has to be clicked
  now. The previous behavior of allowing to do it on the entire row, but
  only if the item was active already, was just too unusual and felt
  weird.
* Reduce margin between chevron icon and the row label.
* Indent child items without chevron some more, otherwise they feel like
  a row on the same level as their parent, just without chevron.
* Fix renaming button taking entire row width. Respect indentation now.
* Fix double-clicking to rename toggling collapsed state on each click.

Some hacks/special-handling was needed so tree-rows always highlight
while the mouse is hovering them, even if the mouse is actually hovering
another button inside the row.

5 files changed, 200 insertions, 63 deletions

diff --git a/source/blender/editors/include/UI_tree_view.hh b/source/blender/editors/include/UI_tree_view.hh
index 7693a833210..272439a2ae9 100644
--- a/source/blender/editors/include/UI_tree_view.hh
+++ b/source/blender/editors/include/UI_tree_view.hh
@@ -138,6 +138,8 @@ class TreeViewLayoutBuilder {
  private:
   /* Created through #TreeViewBuilder. */
   TreeViewLayoutBuilder(uiBlock &block);
+
+  static void polish_layout(const uiBlock &block);
 };
 
 /** \} */
@@ -282,8 +284,7 @@ class AbstractTreeViewItem : public TreeViewItemContainer {
   void begin_renaming();
   void end_renaming();
 
-  const AbstractTreeView &get_tree_view() const;
-  AbstractTreeView &get_tree_view();
+  AbstractTreeView &get_tree_view() const;
   int count_parents() const;
   void deactivate();
   /**
@@ -310,6 +311,8 @@ class AbstractTreeViewItem : public TreeViewItemContainer {
   void ensure_parents_uncollapsed();
   bool matches_including_parents(const AbstractTreeViewItem &other) const;
 
+  uiButTreeRow *tree_row_button();
+
  protected:
   /**
    * Activates this item, deactivates other items, calls the #AbstractTreeViewItem::on_activate()
@@ -323,11 +326,16 @@ class AbstractTreeViewItem : public TreeViewItemContainer {
   static void rename_button_fn(bContext *, void *, char *);
   static AbstractTreeViewItem *find_tree_item_from_rename_button(const uiBut &but);
   static void tree_row_click_fn(struct bContext *, void *, void *);
+  static void collapse_chevron_click_fn(bContext *, void *but_arg1, void *);
+  static bool is_collapse_chevron_but(const uiBut *but);
 
   /** See #AbstractTreeView::change_state_delayed() */
   void change_state_delayed();
+
   void add_treerow_button(uiBlock &block);
-  void add_rename_button(uiBlock &block);
+  void add_indent(uiLayout &row) const;
+  void add_collapse_chevron(uiBlock &block) const;
+  void add_rename_button(uiLayout &row);
 };
 
 /** \} */
@@ -359,9 +367,6 @@ class BasicTreeViewItem : public AbstractTreeViewItem {
    */
   ActivateFn activate_fn_;
 
-  uiBut *button();
-  BIFIconID get_draw_icon() const;
-
  private:
   static void tree_row_click_fn(struct bContext *C, void *arg1, void *arg2);
 
diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c
index 88b23e07f54..92391a703ef 100644
--- a/source/blender/editors/interface/interface.c
+++ b/source/blender/editors/interface/interface.c
@@ -965,7 +965,13 @@ static bool ui_but_update_from_old_block(const bContext *C,
     found_active = true;
   }
   else {
-    const int flag_copy = UI_BUT_DRAG_MULTI;
+    int flag_copy = UI_BUT_DRAG_MULTI;
+
+    /* Stupid special case: The active button may be inside (as in, overlapped on top) a tree-row
+     * button which we also want to keep highlighted then. */
+    if (but->type == UI_BTYPE_TREEROW) {
+      flag_copy |= UI_ACTIVE;
+    }
 
     but->flag = (but->flag & ~flag_copy) | (oldbut->flag & flag_copy);
 
diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c
index f73420b3668..8fdc055a13b 100644
--- a/source/blender/editors/interface/interface_handlers.c
+++ b/source/blender/editors/interface/interface_handlers.c
@@ -4830,14 +4830,23 @@ static int ui_do_but_TREEROW(bContext *C,
   uiButTreeRow *tree_row_but = (uiButTreeRow *)but;
   BLI_assert(tree_row_but->but.type == UI_BTYPE_TREEROW);
 
-  if ((event->type == LEFTMOUSE) && (event->val == KM_DBL_CLICK)) {
-    button_activate_state(C, but, BUTTON_STATE_EXIT);
+  if (data->state == BUTTON_STATE_HIGHLIGHT) {
+    if (event->type == LEFTMOUSE) {
+      if (event->val == KM_CLICK) {
+        button_activate_state(C, but, BUTTON_STATE_EXIT);
+        return WM_UI_HANDLER_BREAK;
+      }
+      else if (event->val == KM_DBL_CLICK) {
+        data->cancel = true;
 
-    UI_tree_view_item_begin_rename(tree_row_but->tree_item);
-    return WM_UI_HANDLER_BREAK;
+        UI_tree_view_item_begin_rename(tree_row_but->tree_item);
+        ED_region_tag_redraw(CTX_wm_region(C));
+        return WM_UI_HANDLER_BREAK;
+      }
+    }
   }
 
-  return ui_do_but_TOG(C, but, data, event);
+  return WM_UI_HANDLER_CONTINUE;
 }
 
 static int ui_do_but_EXIT(bContext *C, uiBut *but, uiHandleButtonData *data, const wmEvent *event)
@@ -9683,6 +9692,38 @@ static int ui_handle_list_event(bContext *C, const wmEvent *event, ARegion *regi
   return retval;
 }
 
+static int ui_handle_tree_hover(const wmEvent *event, const ARegion *region)
+{
+  bool has_treerows = false;
+  LISTBASE_FOREACH (uiBlock *, block, &region->uiblocks) {
+    /* Avoid unnecessary work: Tree-rows are assumed to be inside tree-views. */
+    if (BLI_listbase_is_empty(&block->views)) {
+      continue;
+    }
+
+    LISTBASE_FOREACH (uiBut *, but, &block->buttons) {
+      if (but->type == UI_BTYPE_TREEROW) {
+        but->flag &= ~UI_ACTIVE;
+        has_treerows = true;
+      }
+    }
+  }
+
+  if (!has_treerows) {
+    /* Avoid unnecessary lookup. */
+    return WM_UI_HANDLER_CONTINUE;
+  }
+
+  /* Always highlight the hovered tree-row, even if the mouse hovers another button inside of it.
+   */
+  uiBut *hovered_row_but = ui_tree_row_find_mouse_over(region, event->x, event->y);
+  if (hovered_row_but) {
+    hovered_row_but->flag |= UI_ACTIVE;
+  }
+
+  return WM_UI_HANDLER_CONTINUE;
+}
+
 static void ui_handle_button_return_submenu(bContext *C, const wmEvent *event, uiBut *but)
 {
   uiHandleButtonData *data = but->active;
@@ -11286,6 +11327,10 @@ static int ui_region_handler(bContext *C, const wmEvent *event, void *UNUSED(use
     ui_blocks_set_tooltips(region, true);
   }
 
+  /* Always do this, to reliably update tree-row highlighting, even if the mouse hovers a button
+   * inside the row (it's an overlapping layout). */
+  ui_handle_tree_hover(event, region);
+
   /* delayed apply callbacks */
   ui_apply_but_funcs_after(C);
 
diff --git a/source/blender/editors/interface/tree_view.cc b/source/blender/editors/interface/tree_view.cc
index f3070481da2..5df9c21de4f 100644
--- a/source/blender/editors/interface/tree_view.cc
+++ b/source/blender/editors/interface/tree_view.cc
@@ -19,6 +19,7 @@
  */
 
 #include "DNA_userdef_types.h"
+#include "DNA_windowmanager_types.h"
 
 #include "BKE_context.h"
 
@@ -28,6 +29,8 @@
 
 #include "UI_interface.h"
 
+#include "WM_types.h"
+
 #include "UI_tree_view.hh"
 
 namespace blender::ui {
@@ -88,7 +91,7 @@ void AbstractTreeView::build_layout_from_tree(const TreeViewLayoutBuilder &build
   uiLayout *prev_layout = builder.current_layout();
 
   uiLayout *box = uiLayoutBox(prev_layout);
-  uiLayoutColumn(box, true);
+  uiLayoutColumn(box, false);
 
   foreach_item([&builder](AbstractTreeViewItem &item) { builder.build_row(item); },
                IterOptions::SkipCollapsed);
@@ -172,24 +175,80 @@ void AbstractTreeViewItem::tree_row_click_fn(struct bContext * /*C*/,
                                              void * /*arg2*/)
 {
   uiButTreeRow *tree_row_but = (uiButTreeRow *)but_arg1;
-  BasicTreeViewItem &tree_item = reinterpret_cast<BasicTreeViewItem &>(*tree_row_but->tree_item);
+  AbstractTreeViewItem &tree_item = reinterpret_cast<AbstractTreeViewItem &>(
+      *tree_row_but->tree_item);
 
-  /* Let a click on an opened item activate it, a second click will close it then.
-   * TODO Should this be for asset catalogs only? */
-  if (tree_item.is_collapsed() || tree_item.is_active()) {
-    tree_item.toggle_collapsed();
-  }
   tree_item.activate();
 }
 
 void AbstractTreeViewItem::add_treerow_button(uiBlock &block)
 {
+  /* For some reason a width > (UI_UNIT_X * 2) make the layout system use all available width. */
   tree_row_but_ = (uiButTreeRow *)uiDefBut(
-      &block, UI_BTYPE_TREEROW, 0, "", 0, 0, UI_UNIT_X, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
+      &block, UI_BTYPE_TREEROW, 0, "", 0, 0, UI_UNIT_X * 10, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
 
   tree_row_but_->tree_item = reinterpret_cast<uiTreeViewItemHandle *>(this);
   UI_but_func_set(&tree_row_but_->but, tree_row_click_fn, tree_row_but_, nullptr);
-  UI_but_treerow_indentation_set(&tree_row_but_->but, count_parents());
+}
+
+void AbstractTreeViewItem::add_indent(uiLayout &row) const
+{
+  uiBlock *block = uiLayoutGetBlock(&row);
+  uiLayout *subrow = uiLayoutRow(&row, true);
+  uiLayoutSetFixedSize(subrow, true);
+
+  const float indent_size = count_parents() * UI_DPI_ICON_SIZE;
+  uiDefBut(block, UI_BTYPE_SEPR, 0, "", 0, 0, indent_size, 0, NULL, 0.0, 0.0, 0, 0, "");
+
+  /* Indent items without collapsing icon some more within their parent. Makes it clear that they
+   * are actually nested and not just a row at the same level without a chevron. */
+  if (!is_collapsible() && parent_) {
+    uiDefBut(block, UI_BTYPE_SEPR, 0, "", 0, 0, 0.2f * UI_UNIT_X, 0, NULL, 0.0, 0.0, 0, 0, "");
+  }
+
+  /* Restore. */
+  UI_block_layout_set_current(block, &row);
+}
+
+void AbstractTreeViewItem::collapse_chevron_click_fn(struct bContext *C,
+                                                     void * /*but_arg1*/,
+                                                     void * /*arg2*/)
+{
+  /* There's no data we could pass to this callback. It must be either the button itself or a
+   * consistent address to match buttons over redraws. So instead of passing it somehow, just
+   * lookup the hovered item via context here. */
+
+  const wmWindow *win = CTX_wm_window(C);
+  const ARegion *region = CTX_wm_region(C);
+  uiTreeViewItemHandle *hovered_item_handle = UI_block_tree_view_find_item_at(
+      region, win->eventstate->x, win->eventstate->y);
+  AbstractTreeViewItem *hovered_item = reinterpret_cast<AbstractTreeViewItem *>(
+      hovered_item_handle);
+  BLI_assert(hovered_item != nullptr);
+
+  hovered_item->toggle_collapsed();
+}
+
+bool AbstractTreeViewItem::is_collapse_chevron_but(const uiBut *but)
+{
+  return but->type == UI_BTYPE_BUT_TOGGLE && ELEM(but->icon, ICON_TRIA_RIGHT, ICON_TRIA_DOWN) &&
+         (but->func == collapse_chevron_click_fn);
+}
+
+void AbstractTreeViewItem::add_collapse_chevron(uiBlock &block) const
+{
+  if (!is_collapsible()) {
+    return;
+  }
+
+  const BIFIconID icon = is_collapsed() ? ICON_TRIA_RIGHT : ICON_TRIA_DOWN;
+  uiBut *but = uiDefIconBut(
+      &block, UI_BTYPE_BUT_TOGGLE, 0, icon, 0, 0, UI_UNIT_X, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
+  /* Note that we're passing the tree-row button here, not the chevron one. */
+  UI_but_func_set(but, collapse_chevron_click_fn, nullptr, nullptr);
+
+  /* Check if the query for the button matches the created button. */
+  BLI_assert(is_collapse_chevron_but(but));
 }
 
 AbstractTreeViewItem *AbstractTreeViewItem::find_tree_item_from_rename_button(
@@ -226,16 +285,23 @@ void AbstractTreeViewItem::rename_button_fn(bContext *UNUSED(C), void *arg, char
   item->end_renaming();
 }
 
-void AbstractTreeViewItem::add_rename_button(uiBlock &block)
+void AbstractTreeViewItem::add_rename_button(uiLayout &row)
 {
+  uiBlock *block = uiLayoutGetBlock(&row);
+  eUIEmbossType previous_emboss = UI_block_emboss_get(block);
+
+  uiLayoutRow(&row, false);
+  /* Enable emboss for the text button. */
+  UI_block_emboss_set(block, UI_EMBOSS);
+
   AbstractTreeView &tree_view = get_tree_view();
-  uiBut *rename_but = uiDefBut(&block,
+  uiBut *rename_but = uiDefBut(block,
                                UI_BTYPE_TEXT,
                                1,
                                "",
                                0,
                                0,
-                               UI_UNIT_X,
+                               UI_UNIT_X * 10,
                                UI_UNIT_Y,
                                tree_view.rename_buffer_->data(),
                                1.0f,
@@ -248,12 +314,15 @@ void AbstractTreeViewItem::add_rename_button(uiBlock &block)
    * callback is executed. */
   UI_but_func_rename_set(rename_but, AbstractTreeViewItem::rename_button_fn, rename_but);
 
-  const bContext *evil_C = static_cast<bContext *>(block.evil_C);
+  const bContext *evil_C = static_cast<bContext *>(block->evil_C);
   ARegion *region = CTX_wm_region(evil_C);
   /* Returns false if the button was removed. */
-  if (UI_but_active_only(evil_C, region, &block, rename_but) == false) {
+  if (UI_but_active_only(evil_C, region, block, rename_but) == false) {
     end_renaming();
   }
+
+  UI_block_emboss_set(block, previous_emboss);
+  UI_block_layout_set_current(block, &row);
 }
 
 void AbstractTreeViewItem::on_activate()
@@ -335,12 +404,7 @@ void AbstractTreeViewItem::end_renaming()
   tree_view.rename_buffer_ = nullptr;
 }
 
-const AbstractTreeView &AbstractTreeViewItem::get_tree_view() const
-{
-  return static_cast<AbstractTreeView &>(*root_);
-}
-
-AbstractTreeView &AbstractTreeViewItem::get_tree_view()
+AbstractTreeView &AbstractTreeViewItem::get_tree_view() const
 {
   return static_cast<AbstractTreeView &>(*root_);
 }
@@ -454,6 +518,11 @@ bool AbstractTreeViewItem::matches_including_parents(const AbstractTreeViewItem
   return true;
 }
 
+uiButTreeRow *AbstractTreeViewItem::tree_row_button()
+{
+  return tree_row_but_;
+}
+
 void AbstractTreeViewItem::change_state_delayed()
 {
   if (is_active_fn_()) {
@@ -481,25 +550,56 @@ TreeViewLayoutBuilder::TreeViewLayoutBuilder(uiBlock &block) : block_(block)
 {
 }
 
-void TreeViewLayoutBuilder::build_row(AbstractTreeViewItem &item) const
+/**
+ * Moves the button following the last added chevron closer to the list item.
+ *
+ * Iterates backwards over buttons until finding the tree-row button, which is assumed to be the
+ * first button added for the row, and can act as a delimiter that way.
+ */
+void TreeViewLayoutBuilder::polish_layout(const uiBlock &block)
 {
-  uiLayout *prev_layout = current_layout();
-  uiLayout *row = uiLayoutRow(prev_layout, false);
+  LISTBASE_FOREACH_BACKWARD (uiBut *, but, &block.buttons) {
+    if (AbstractTreeViewItem::is_collapse_chevron_but(but) && but->next &&
+        /* Embossed buttons with padding-less text padding look weird, so don't touch them. */
+        ELEM(but->next->emboss, UI_EMBOSS_NONE, UI_EMBOSS_NONE_OR_STATUS)) {
+      UI_but_drawflag_enable(static_cast<uiBut *>(but->next), UI_BUT_NO_TEXT_PADDING);
+    }
 
-  uiLayoutOverlap(row);
+    if (but->type == UI_BTYPE_TREEROW) {
+      break;
+    }
+  }
+}
 
+void TreeViewLayoutBuilder::build_row(AbstractTreeViewItem &item) const
+{
   uiBlock &block_ = block();
 
+  uiLayout *prev_layout = current_layout();
+  eUIEmbossType previous_emboss = UI_block_emboss_get(&block_);
+
+  uiLayout *overlap = uiLayoutOverlap(prev_layout);
+
+  uiLayoutRow(overlap, false);
   /* Every item gets one! Other buttons can be overlapped on top. */
   item.add_treerow_button(block_);
 
+  /* After adding tree-row button (would disable hover highlighting). */
+  UI_block_emboss_set(&block_, UI_EMBOSS_NONE);
+
+  uiLayout *row = uiLayoutRow(overlap, true);
+  item.add_indent(*row);
+  item.add_collapse_chevron(block_);
+
   if (item.is_renaming()) {
-    item.add_rename_button(block_);
+    item.add_rename_button(*row);
   }
   else {
     item.build_row(*row);
   }
+  polish_layout(block_);
 
+  UI_block_emboss_set(&block_, previous_emboss);
   UI_block_layout_set_current(&block_, prev_layout);
 }
 
@@ -520,12 +620,9 @@ BasicTreeViewItem::BasicTreeViewItem(StringRef label, BIFIconID icon_) : icon(ic
   label_ = label;
 }
 
-void BasicTreeViewItem::build_row(uiLayout & /*row*/)
+void BasicTreeViewItem::build_row(uiLayout &row)
 {
-  if (BIFIconID icon = get_draw_icon()) {
-    ui_def_but_icon(&tree_row_but_->but, icon, UI_HAS_ICON);
-  }
-  tree_row_but_->but.str = BLI_strdupn(label_.c_str(), label_.length());
+  uiItemL(&row, label_.c_str(), icon);
 }
 
 void BasicTreeViewItem::on_activate()
@@ -540,24 +637,6 @@ void BasicTreeViewItem::on_activate(ActivateFn fn)
   activate_fn_ = fn;
 }
 
-BIFIconID BasicTreeViewItem::get_draw_icon() const
-{
-  if (icon) {
-    return icon;
-  }
-
-  if (is_collapsible()) {
-    return is_collapsed() ? ICON_TRIA_RIGHT : ICON_TRIA_DOWN;
-  }
-
-  return ICON_NONE;
-}
-
-uiBut *BasicTreeViewItem::button()
-{
-  return &tree_row_but_->but;
-}
-
 }  // namespace blender::ui
 
 using namespace blender::ui;
diff --git a/source/blender/editors/space_file/asset_catalog_tree_view.cc b/source/blender/editors/space_file/asset_catalog_tree_view.cc
index fac38e71220..84bfa58be85 100644
--- a/source/blender/editors/space_file/asset_catalog_tree_view.cc
+++ b/source/blender/editors/space_file/asset_catalog_tree_view.cc
@@ -223,11 +223,12 @@ void AssetCatalogTreeViewItem::build_row(uiLayout &row)
     return;
   }
 
+  uiButTreeRow *tree_row_but = tree_row_button();
   PointerRNA *props;
   const CatalogID catalog_id = catalog_item_.get_catalog_id();
 
   props = UI_but_extra_operator_icon_add(
-      button(), "ASSET_OT_catalog_new", WM_OP_INVOKE_DEFAULT, ICON_ADD);
+      (uiBut *)tree_row_but, "ASSET_OT_catalog_new", WM_OP_INVOKE_DEFAULT, ICON_ADD);
   RNA_string_set(props, "parent_path", catalog_item_.catalog_path().c_str());
 
   /* Tree items without a catalog ID represent components of catalog paths that are not
@@ -238,7 +239,7 @@ void AssetCatalogTreeViewItem::build_row(uiLayout &row)
     BLI_uuid_format(catalog_id_str_buffer, catalog_id);
 
     props = UI_but_extra_operator_icon_add(
-        button(), "ASSET_OT_catalog_delete", WM_OP_INVOKE_DEFAULT, ICON_X);
+        (uiBut *)tree_row_but, "ASSET_OT_catalog_delete", WM_OP_INVOKE_DEFAULT, ICON_X);
     RNA_string_set(props, "catalog_id", catalog_id_str_buffer);
   }
 }
@@ -301,6 +302,7 @@ bool AssetCatalogTreeViewItem::drop_into_catalog(const AssetCatalogTreeView &tre
     /* Trigger re-run of filtering to update visible assets. */
     filelist_tag_needs_filtering(tree_view.space_file_.files);
     file_select_deselect_all(&tree_view.space_file_, FILE_SEL_SELECTED | FILE_SEL_HIGHLIGHTED);
+    WM_main_add_notifier(NC_SPACE | ND_SPACE_FILE_LIST, nullptr);
   }
 
   return true;
@@ -341,7 +343,7 @@ void AssetCatalogTreeViewAllItem::build_row(uiLayout &row)
 
   PointerRNA *props;
   props = UI_but_extra_operator_icon_add(
-      button(), "ASSET_OT_catalog_new", WM_OP_INVOKE_DEFAULT, ICON_ADD);
+      (uiBut *)tree_row_button(), "ASSET_OT_catalog_new", WM_OP_INVOKE_DEFAULT, ICON_ADD);
   /* No parent path to use the root level. */
   RNA_string_set(props, "parent_path", nullptr);
 }