Fix #104722: Outliner Renaming Double Click Error #105872

Kaicheng Zhou · 2023-03-18T05:50:38+01:00

Kaicheng Zhou commented

2023-03-18 05:50:38 +01:00

First-time contributor

Problem
After renaming an item by F2 key, double clicking on an item will result in renaming the active item, rather the hovered one.
It's becuase use_active keeps being true since pressing F2, and the code always selects the active item when use_active is true.

Solution
I tried to make minimal change. Before selecting the item to be renamed, check the event type. Select the hovered item when it's a click-caused renaming.

**Problem** After renaming an item by F2 key, double clicking on an item will result in renaming the active item, rather the hovered one. It's becuase `use_active` keeps being `true` since pressing F2, and the code always selects the active item when `use_active` is `true`. **Solution** I tried to make minimal change. Before selecting the item to be renamed, check the event type. Select the hovered item when it's a click-caused renaming.

Kaicheng Zhou added 1 commit 2023-03-18 05:50:51 +01:00

5b9bd78423 Fix #104722 : Outliner Renaming Double Click Error

Pratik Borhade requested changes 2023-04-22 08:12:13 +02:00

Pratik Borhade left a comment

Hi, thanks for the PR. I doubt whether checking event-type is correct solution here.
double-click to rename is not hardcoded. This can be changed from prefs. If you replace double-click with ctrl-alt-RMB, patch will fail.
I think correct fix will be to set "use_active=false" in blender_defaults.py (same for industry compatible keymap):

diff --git a/scripts/presets/keyconfig/keymap_data/blender_default.py b/scripts/presets/keyconfig/keymap_data/blender_default.py
index 22f4875d0cb..60cc7b63ab7 100644
--- a/scripts/presets/keyconfig/keymap_data/blender_default.py
+++ b/scripts/presets/keyconfig/keymap_data/blender_default.py
@@ -1122,7 +1122,8 @@ def km_outliner(params):

    items.extend([
        ("outliner.highlight_update", {"type": 'MOUSEMOVE', "value": 'ANY', "any": True}, None),
-        ("outliner.item_rename", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK'}, None),
+        ("outliner.item_rename", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK'},
+         {"properties": [("use_active", False)]}),
        ("outliner.item_rename", {"type": 'F2', "value": 'PRESS'},
         {"properties": [("use_active", True)]}),
        ("outliner.item_activate", {"type": 'LEFTMOUSE', "value": 'CLICK'},

Hi, thanks for the PR. I doubt whether checking event-type is correct solution here. double-click to rename is not hardcoded. This can be changed from prefs. If you replace double-click with `ctrl-alt-RMB`, patch will fail. I think correct fix will be to set "use_active=false" in `blender_defaults.py` (same for industry compatible keymap): ```Diff diff --git a/scripts/presets/keyconfig/keymap_data/blender_default.py b/scripts/presets/keyconfig/keymap_data/blender_default.py index 22f4875d0cb..60cc7b63ab7 100644 --- a/scripts/presets/keyconfig/keymap_data/blender_default.py +++ b/scripts/presets/keyconfig/keymap_data/blender_default.py @@ -1122,7 +1122,8 @@ def km_outliner(params): items.extend([ ("outliner.highlight_update", {"type": 'MOUSEMOVE', "value": 'ANY', "any": True}, None), - ("outliner.item_rename", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK'}, None), + ("outliner.item_rename", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK'}, + {"properties": [("use_active", False)]}), ("outliner.item_rename", {"type": 'F2', "value": 'PRESS'}, {"properties": [("use_active", True)]}), ("outliner.item_activate", {"type": 'LEFTMOUSE', "value": 'CLICK'}, ```

Pratik Borhade commented

2023-04-26 18:08:07 +02:00

@Harley / @JulianEisel , is suggested fix correct?
Maybe I can push it directly (or open a new PR) since the suggested fix is completely different than the original attempt.

@Harley / @JulianEisel , is suggested fix correct? Maybe I can push it directly (or open a new PR) since the suggested fix is completely different than the original attempt.

Julian Eisel commented

2023-04-26 21:05:35 +02:00

I can't try this right now, but if I understand the issue correctly, this should work:

diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc
index bf0178d7dec..815bf6a7f7b 100644
--- a/source/blender/editors/space_outliner/outliner_edit.cc
+++ b/source/blender/editors/space_outliner/outliner_edit.cc
@@ -423,6 +423,8 @@ static int outliner_item_rename(bContext *C, wmOperator *op, const wmEvent *even
 
 void OUTLINER_OT_item_rename(wmOperatorType *ot)
 {
+  PropertyRNA *prop;
+
   ot->name = "Rename";
   ot->idname = "OUTLINER_OT_item_rename";
   ot->description = "Rename the active element";
@@ -434,11 +436,12 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot)
   /* Flags. */
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
 
-  RNA_def_boolean(ot->srna,
-                  "use_active",
-                  false,
-                  "Use Active",
-                  "Rename the active item, rather than the one the mouse is over");
+  prop = RNA_def_boolean(ot->srna,
+                         "use_active",
+                         false,
+                         "Use Active",
+                         "Rename the active item, rather than the one the mouse is over");
+  RNA_def_property_flag(prop, PROP_SKIP_SAVE);
 }
 
 /** \} */

(Should probably also set PROP_HIDDEN).

Otherwise you can go ahead and commit your fix @PratikPB2123 .

I can't try this right now, but if I understand the issue correctly, this should work: ```Diff diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc index bf0178d7dec..815bf6a7f7b 100644 --- a/source/blender/editors/space_outliner/outliner_edit.cc +++ b/source/blender/editors/space_outliner/outliner_edit.cc @@ -423,6 +423,8 @@ static int outliner_item_rename(bContext *C, wmOperator *op, const wmEvent *even void OUTLINER_OT_item_rename(wmOperatorType *ot) { + PropertyRNA *prop; + ot->name = "Rename"; ot->idname = "OUTLINER_OT_item_rename"; ot->description = "Rename the active element"; @@ -434,11 +436,12 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot) /* Flags. */ ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; - RNA_def_boolean(ot->srna, - "use_active", - false, - "Use Active", - "Rename the active item, rather than the one the mouse is over"); + prop = RNA_def_boolean(ot->srna, + "use_active", + false, + "Use Active", + "Rename the active item, rather than the one the mouse is over"); + RNA_def_property_flag(prop, PROP_SKIP_SAVE); } /** \} */ ``` (Should probably also set `PROP_HIDDEN`). Otherwise you can go ahead and commit your fix @PratikPB2123 .

Pratik Borhade commented

2023-04-27 03:50:38 +02:00

Yes, this works too and looks more logical :)

Harley Acheson commented

2023-05-01 00:10:42 +02:00

Julian's solution works perfectly and seems a better fit. Tested as:

diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc
index 4ea83f4720c..55ce166720f 100644
--- a/source/blender/editors/space_outliner/outliner_edit.cc
+++ b/source/blender/editors/space_outliner/outliner_edit.cc
@@ -434,11 +434,13 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot)
   /* Flags. */
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
 
-  RNA_def_boolean(ot->srna,
-                  "use_active",
-                  false,
-                  "Use Active",
-                  "Rename the active item, rather than the one the mouse is over");
+  PropertyRNA *prop = RNA_def_boolean(
+      ot->srna,
+      "use_active",
+      false,
+      "Use Active",
+      "Rename the active item, rather than the one the mouse is over");
+  RNA_def_property_flag(prop, (PropertyFlag)(PROP_SKIP_SAVE | PROP_HIDDEN));
 }
 
 /** \} */

Julian's solution works **perfectly** and seems a better fit. Tested as: ```Diff diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc index 4ea83f4720c..55ce166720f 100644 --- a/source/blender/editors/space_outliner/outliner_edit.cc +++ b/source/blender/editors/space_outliner/outliner_edit.cc @@ -434,11 +434,13 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot) /* Flags. */ ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; - RNA_def_boolean(ot->srna, - "use_active", - false, - "Use Active", - "Rename the active item, rather than the one the mouse is over"); + PropertyRNA *prop = RNA_def_boolean( + ot->srna, + "use_active", + false, + "Use Active", + "Rename the active item, rather than the one the mouse is over"); + RNA_def_property_flag(prop, (PropertyFlag)(PROP_SKIP_SAVE | PROP_HIDDEN)); } /** \} */ ```

Julian Eisel referenced this issue from a commit

2023-05-01 11:04:04 +02:00

Fix #104722: Outliner renaming uses wrong item

Julian Eisel commented

2023-05-01 11:05:28 +02:00

Committed f8d2156dda now. Thanks all!

Committed f8d2156dda now. Thanks all!

Julian Eisel closed this pull request

2023-05-01 11:05:30 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #104722: Outliner Renaming Double Click Error #105872

Pull request closed