Fix: Show modal keymap for operators called inside macros #108027

Merged

Harley Acheson merged 4 commits from lone_noel/blender:macro-operator-modal-shortcuts into main

2023-09-15 19:33:27 +02:00

Leon Schittek commented

2023-05-17 23:25:26 +02:00

Member

Display the modal keymap in the status bar for modal operators that
are called within macros. Most notably, this means the keymap for the
the transform operator is now displayed, when it's called as part of
the duplicate, extrude, or loopcut operator or when moving nodes.

Since the macro operator itself does not have a modal keymap, the
status bar would always use the fallback. This is fixed by explicitly
checking for the currently running operator inside a macro.

Additionally this PR also adjust the header text displayed while
transforming nodes so it's not too redundant.

Comparison

	main	fix
duplicate
moving nodes

Display the modal keymap in the status bar for modal operators that are called within macros. Most notably, this means the keymap for the the transform operator is now displayed, when it's called as part of the duplicate, extrude, or loopcut operator or when moving nodes. Since the macro operator itself does not have a modal keymap, the status bar would always use the fallback. This is fixed by explicitly checking for the currently running operator inside a macro. --- Additionally this PR also adjust the header text displayed while transforming nodes so it's not too redundant. **Comparison** ||main|fix| |---|---|---| |duplicate|![main_duplicate.png](/attachments/25c24175-e0c2-4cce-9119-6620878acf89)|![fix_duplicate.png](/attachments/6958092f-707b-46d8-a5bf-3f4e20540380)| |moving nodes|![main_node-editor.png](/attachments/df2b594d-90b5-46ba-8e06-f17281272784)|![fix_node-editor.png](/attachments/e8926c2d-a666-4ace-afdd-7213c53cb9db)|

main_node-editor.png

79 KiB

fix_node-editor.png

90 KiB

main_duplicate.png

472 KiB

fix_duplicate.png

486 KiB

Leon Schittek added the

Nodes & Physics

labels 2023-05-17 23:25:26 +02:00

Leon Schittek added 2 commits 2023-05-17 23:25:37 +02:00

16c213f415 Fix: Show modal keymap for operators in macros

21f97997f2 Node Editor: Remove redundant header text during transform.

Now that the modal shortcuts are shown in the status bar, they
don't have to be repeated in the editor's header.

Iliya Katushenock added this to the Nodes & Physics project 2023-05-18 06:36:31 +02:00

Hans Goudey requested review from Campbell Barton 2023-05-31 22:39:24 +02:00

Hans Goudey modified the project from Nodes & Physics to User Interface

2023-06-06 18:47:16 +02:00

Harley Acheson requested review from Harley Acheson 2023-06-07 20:02:04 +02:00

Harley Acheson commented

2023-06-07 20:03:41 +02:00

Member

This works great!

This works great!

Harley Acheson approved these changes 2023-06-07 20:04:28 +02:00

Leon Schittek commented

2023-06-14 07:57:26 +02:00

Author

Member

What is the etiquette when there are multiple reviewers? Is this good to go or should I wait until @ideasman42 had a chance to take a look?

What is the etiquette when there are multiple reviewers? Is this good to go or should I wait until @ideasman42 had a chance to take a look?

Harley Acheson commented

2023-06-14 16:48:17 +02:00

Member

What is the etiquette when there are multiple reviewers? Is this good to go or should I wait until @ideasman42 had a chance to take a look?

Sorry I should have mentioned that. For this one we really should treat Campbell as a blocking reviewer.

> What is the etiquette when there are multiple reviewers? Is this good to go or should I wait until @ideasman42 had a chance to take a look? Sorry I should have mentioned that. For this one we really should treat Campbell as a blocking reviewer.

Leon Schittek added 1 commit 2023-08-19 18:17:10 +02:00

30e100f81b Merge branch 'main' into macro-operator-modal-shortcuts

Harley Acheson added this to the 4.0 milestone 2023-08-24 18:29:09 +02:00

Leon Schittek added 1 commit 2023-09-15 15:13:17 +02:00

1bd7dbe8eb Merge branch 'main' into macro-operator-modal-shortcuts

Harley Acheson commented

2023-09-15 18:05:08 +02:00

Member

The code changes in WM_window_modal_keymap_status_draw make sense.

I mostly have questions about the header status text changes. But please just take these as questions as your changes are probably great.

I understand the removal of the keymap string from "Toggle auto-offset direction" as that is shown on the status bar. We have keymaps on the status bar and status in the header.

But then "Toggle auto-attach" is removed from the header. Shouldn't that be similarly changed to show the current status of it? These two things look similar but are treated differently. I realize the current status of that is shown on the status bar, so is that a different problem in that some keymap toggle statuses are shown there and others not?

The code changes in `WM_window_modal_keymap_status_draw` make sense. I mostly have questions about the header status text changes. But please just take these as questions as your changes are probably great. I understand the removal of the keymap string from "Toggle auto-offset direction" as that is shown on the status bar. We have keymaps on the status bar and status in the header. But then "Toggle auto-attach" is removed from the header. Shouldn't that be similarly changed to show the current status of it? These two things look similar but are treated differently. I realize the current status of that is shown on the status bar, so is that a different problem in that some keymap toggle statuses are shown there and others not?

Leon Schittek commented

2023-09-15 19:21:09 +02:00

Author

Member

But then "Toggle auto-attach" is removed from the header. Shouldn't that be similarly changed to show the current status of it? These two things look similar but are treated differently.

That's actually a good question. To me it's mostly a difference in how these two toggles work and are indicated otherwise.

The auto attachment is always active and only deactivated while pressing alt.
So the state (off) and the action (pressing the key) are coupled unambiguously. Additionally the node attchment is telegraphed by highlighting the link so it's obvious when the attachment is off, eliminating the need for an additional indicator, I guess,

For the direction on the other hand pressing T actually toggles between left and right (you need to release and press again to toggle back).
Currently the effects of this can only be seen after confirming the transform of nodes, so without an indicator the behavior wouldn't be predictable. Ideally there would be a better visual indicator directly in the node editor for this, rather than just this text in the header...

I realize the current status of that is shown on the status bar, so is that a different problem in that some keymap toggle statuses are shown there and others not?

That it actually says "Node Attachment (Off)" is a bit of an implementation quirk of these "toggle while pressed" actions (like Node Attachment or Snap Invert).
They use two modal keymap items to do this: one to turn the property on and one to turn it off.
In this case the modal keymap has the "Node Attachment" item that is set to be triggered when Alt is released and the "Node Attachment (Off)" item that is triggered when Alt is pressed.
Events on release don't seem to be shown in the status bar so that's why only "Node Attachment (Off)" one is shown, which also matches the state it toggles.

> But then "Toggle auto-attach" is removed from the header. Shouldn't that be similarly changed to show the current status of it? These two things look similar but are treated differently. That's actually a good question. To me it's mostly a difference in how these two toggles work and are indicated otherwise. The auto attachment is always active and only deactivated *while* pressing `alt`. So the state (off) and the action (pressing the key) are coupled unambiguously. Additionally the node attchment is telegraphed by highlighting the link so it's obvious when the attachment is off, eliminating the need for an additional indicator, I guess, For the direction on the other hand pressing `T` actually toggles between left and right (you need to release and press again to toggle back). Currently the effects of this can only be seen after confirming the transform of nodes, so without an indicator the behavior wouldn't be predictable. Ideally there would be a better visual indicator directly in the node editor for this, rather than just this text in the header... >I realize the current status of that is shown on the status bar, so is that a different problem in that some keymap toggle statuses are shown there and others not? That it actually says "Node Attachment **(Off)**" is a bit of an implementation quirk of these "toggle while pressed" actions (like Node Attachment or Snap Invert). They use two modal keymap items to do this: one to turn the property on and one to turn it off. In this case the modal keymap has the "Node Attachment" item that is set to be triggered when `Alt` is *released* and the "Node Attachment (Off)" item that is triggered when `Alt` is *pressed*. Events on release don't seem to be shown in the status bar so that's why only "Node Attachment (Off)" one is shown, which also matches the state it toggles.

Harley Acheson commented

2023-09-15 19:28:57 +02:00

Member

Thanks for the detailed explanation. I mostly wanted to make sure it was thought through, and you certainly have.

@lone_noel Thanks for the detailed explanation. I mostly wanted to make sure it was thought through, and you certainly have.

Harley Acheson merged commit 42f8b49581 into main

2023-09-15 19:33:27 +02:00

Harley Acheson referenced this issue from a commit

2023-09-15 19:33:28 +02:00

Fix: Show modal keymap for operators called inside macros

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#108027

No description provided.