Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inspector direct editing #6257

Merged

Conversation

murraystevenson
Copy link
Contributor

This adds support for "direct" editing to the private Inspector API with new edit() and canEdit() methods on Inspector::Result that allow creating an edit directly from a value and checking whether an edit could be made from a value.

This PR includes two usages of this API, one which allows us to simplify the toggling of bool values in _InspectorColumn (and remove SetMembershipInspector::editSetMembership() in the process). The other brings drag and drop editing to the AttributeEditor, LightEditor and RenderPassEditor.

editorDragAndDrop

We also plan to use this API to provide copy/paste editing in the same editors in a future PR.

@murraystevenson murraystevenson self-assigned this Feb 5, 2025
@murraystevenson murraystevenson force-pushed the inspectorColumnDrag branch 2 times, most recently from c952758 to babefb3 Compare February 5, 2025 02:54
Copy link
Member

@johnhaddon johnhaddon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Murray - this is another nice step forwards for the editors. We already chatted a bit about possible refactoring of the Inspector::Result methods so they're a bit more uniform - I've made a few comments in that regard, but I don't know if now is the time to address them. Perhaps you'd find it easier to get this over the line and then revisit? Since it's private I think that would be OK if it helps. Other comments inline as usual...
Cheers...
John

include/GafferSceneUI/Private/Inspector.h Show resolved Hide resolved
include/GafferSceneUI/Private/Inspector.h Show resolved Hide resolved
src/GafferSceneUIModule/InspectorBinding.cpp Show resolved Hide resolved
src/GafferSceneUI/Inspector.cpp Outdated Show resolved Hide resolved
src/GafferSceneUI/SetMembershipInspector.cpp Show resolved Hide resolved
python/GafferUI/PathListingWidget.py Outdated Show resolved Hide resolved
python/GafferUI/PathListingWidget.py Show resolved Hide resolved
python/GafferSceneUI/_InspectorColumn.py Outdated Show resolved Hide resolved
python/GafferSceneUI/_InspectorColumn.py Outdated Show resolved Hide resolved
python/GafferSceneUI/_InspectorColumn.py Outdated Show resolved Hide resolved
We'll be introducing a new editFunction for making direct edits.
@murraystevenson
Copy link
Contributor Author

Thanks for the input, I've addressed the drag and drop related comments inline but will punt the Inspector API related suggestions to a later PR as it feels like some broader reshuffling is necessary now that we have the required functionality in place...

I've added one additional commit on the end which ensures edits on TweakPlugs are made in Create mode. This seems like the most consistent behaviour for an interaction like drag and drop or copy / paste. cf766b8

@johnhaddon
Copy link
Member

Thanks Murray, LGTM! Squash and merge away...

This simplifies things and allows us to no longer rely on `SetMembershipInspector::editSetMembership()`
This is replaced by Inspector's more generic `edit()` and `canEdit()`
The PathListingWidget is just the intermediary, we expect the column to do the work. Signals are forwarded to the column such that dragEnter and dragLeave are emitted for each index.
This avoids adding new member variables to PathColumn for Gaffer 1.5 and should be reverted on main.
@murraystevenson murraystevenson merged commit a3613e1 into GafferHQ:1.5_maintenance Feb 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants