Details

Reviewers

Maniphest Tasks

T92965: Refactor the spreadsheet data set region to use the abstract tree view API

Commits

rB01df48a98394: Refactor: Port spreadsheet data set to UI tree view

Summary

This patch removes a bunch of specific code for drawing the spreadsheet
data set region, which was an overly specific solution for a generic UI.
Nowadays, the UI tree view API used for asset browser catalogs is a much
better way to implement this behavior.

The missing padding in the screenshot has been fixed in the latest version of the patch

Diff Detail

Repository

rB Blender

Branch

cleanup-spreadsheet-tree-view (branched from master)

Build Status

Buildable 18820
Build 18820: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Nov 12 2021, 5:22 AM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B18611: Diff 44660.Nov 12 2021, 5:22 AM

I'd like to ask a question or two tomorrow, then resolving the TODO should be quick.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Nov 12 2021, 5:23 AM

Hans Goudey (HooglyBoogly) added projects: Geometry Nodes, User Interface.

Hans Goudey (HooglyBoogly) added a parent revision: D13197: Fix: Exception in tree view code in new case.

Hans Goudey (HooglyBoogly) added a task: T92965: Refactor the spreadsheet data set region to use the abstract tree view API.

Julian Eisel (Severin) awarded a token.Nov 12 2021, 10:19 AM

Remove collapsing, active check

Harbormaster completed remote builds in B18632: Diff 44692.Nov 12 2021, 6:58 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Nov 12 2021, 6:59 PM

Merge branch 'master' into cleanup-spreadsheet-tree-view

Harbormaster completed remote builds in B18815: Diff 45004.Nov 19 2021, 6:42 PM

Very nice to see this! And I really think you can see the code improvement here, it's just much more direct and to the point.

This currently breaks collapsing of catalogs, noted a proposed solution inline.
Another issue is that this could use some padding for the right aligned text, as you can see in your screenshot:

You could use tree_row_button() to get the underlying tree-row button and set the hint text for that (UI_but_hint_drawstr_set()). For the right aligned icons in the asset catalogs I do this similarly, it uses extra operator-icons on the tree-row button. This just seems to work better than other things I've tried.

source/blender/editors/interface/interface.c
6927	The `UI_but_datasetrow_xxx()` function declarations are still in the header.
source/blender/editors/interface/tree_view.cc
360	I think in a hierarchical UI it's more common to want collapsing than to not want it. So would say this should return `true` by default.
source/blender/editors/space_spreadsheet/space_spreadsheet.cc
695–701	I'd prefer having a `spreadsheet_panels.cc` for this, similar to `file_panels.c`.
source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw_geometry.hh
28 ↗	(On Diff #45004)	Any reason to have a header for this? It's not needed strictly speaking, but am also not against having it (doesn't hurt).

Julian Eisel (Severin) requested changes to this revision.Nov 19 2021, 8:16 PM

This revision now requires changes to proceed.Nov 19 2021, 8:16 PM

Move panel registration to a new file
Remove old functions from header
Make tree view items collapsible by default
Remove unnecessary header
Fix padding (use UI_but_hint_drawstr_set to draw number)

Thanks for the review!

source/blender/editors/interface/interface.c
6927	Oops, thanks!
source/blender/editors/interface/tree_view.cc
360	Good point.
source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw_geometry.hh
28 ↗	(On Diff #45004)	Hmm, no, I guess there isn't. I was having trouble with forward declaration, but this doesn't seem to be the solution.

Harbormaster completed remote builds in B18820: Diff 45011.Nov 19 2021, 9:09 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Nov 19 2021, 10:36 PM

Julian Eisel (Severin) accepted this revision.Nov 19 2021, 11:08 PM

This revision is now accepted and ready to land.Nov 19 2021, 11:08 PM

Closed by commit rB01df48a98394: Refactor: Port spreadsheet data set to UI tree view (authored by Hans Goudey (HooglyBoogly)). · Explain WhyNov 19 2021, 11:37 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rB01df48a98394: Refactor: Port spreadsheet data set to UI tree view.

Refactor: Port spreadsheet data set to UI tree view
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 45011

source/blender/editors/include/UI_interface.h

source/blender/editors/include/UI_tree_view.hh

source/blender/editors/interface/interface.c

source/blender/editors/interface/interface_handlers.c

source/blender/editors/interface/interface_intern.h

source/blender/editors/interface/interface_query.c

source/blender/editors/interface/interface_widgets.c

source/blender/editors/interface/tree_view.cc

source/blender/editors/space_file/asset_catalog_tree_view.cc

source/blender/editors/space_spreadsheet/CMakeLists.txt

source/blender/editors/space_spreadsheet/space_spreadsheet.cc

source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc

source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw.hh

source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw.cc

source/blender/editors/space_spreadsheet/spreadsheet_dataset_layout.hh

source/blender/editors/space_spreadsheet/spreadsheet_dataset_layout.cc

source/blender/editors/space_spreadsheet/spreadsheet_intern.hh

source/blender/editors/space_spreadsheet/spreadsheet_panels.cc

Refactor: Port spreadsheet data set to UI tree viewClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 45011

source/blender/editors/include/UI_interface.h

source/blender/editors/include/UI_tree_view.hh

source/blender/editors/interface/interface.c

source/blender/editors/interface/interface_handlers.c

source/blender/editors/interface/interface_intern.h

source/blender/editors/interface/interface_query.c

source/blender/editors/interface/interface_widgets.c

source/blender/editors/interface/tree_view.cc

source/blender/editors/space_file/asset_catalog_tree_view.cc

source/blender/editors/space_spreadsheet/CMakeLists.txt

source/blender/editors/space_spreadsheet/space_spreadsheet.cc

source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc

source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw.hh

source/blender/editors/space_spreadsheet/spreadsheet_dataset_draw.cc

source/blender/editors/space_spreadsheet/spreadsheet_dataset_layout.hh

source/blender/editors/space_spreadsheet/spreadsheet_dataset_layout.cc

source/blender/editors/space_spreadsheet/spreadsheet_intern.hh

source/blender/editors/space_spreadsheet/spreadsheet_panels.cc

Refactor: Port spreadsheet data set to UI tree view
ClosedPublic
Actions

Revision Contents
Changeset List