Consolidation of Operator Dialogs #117489

New Issue

Harley Acheson · 2024-01-24T21:46:03+01:00

Harley Acheson commented

2024-01-24 21:46:03 +01:00

I am quite happy with our new ability to customize confirmation dialogs. It allows a lot of configurability that seems to address all the different ways we might want to use those. So I'd like to consider extending that configurability into other operator dialogs, making them more customizable, and remove some redundancy.

At the core these things are quite similar. If the Confirmation dialog is given the (optional) ability to include operator properties we can - if we want - get things like this:

And if we use the confirmation dialog in place of the Operator Properties Dialog we get things like this:

To consolidate these things I propose the following:

The Confirmation dialog needs to able work nicely without an icon. Simple enough.

Non-confirmation operator popups currently pass around a wmOpPopUp which contains only the following:

struct wmOpPopUp {
  wmOperator *op;
  int width;
  int height;
  int free_op;
};

We can just add these four items to the larger wmConfirmDetails (with width becoming min_width) and then rename wmConfirmDetails to wmOpPopUp. And then also add an include_properties bool.

We should eventually end up with all dialogs that are much more configurable and sharing more common code.

I am quite happy with our new ability to customize confirmation dialogs. It allows a lot of configurability that seems to address all the different ways we might want to use those. So I'd like to consider _extending_ that configurability into other operator dialogs, making them more customizable, and remove some redundancy. At the core these things are quite similar. If the Confirmation dialog is given the (optional) ability to include operator properties we can - if we want - get things like this: ![image](/attachments/e5164dcd-a6bd-4d5d-8086-7da0667aa524) And if we use the confirmation dialog in place of the Operator Properties Dialog we get things like this: ![image](/attachments/9d7095ef-b540-453e-bc91-9e9f9ff32df8) To consolidate these things I propose the following: The Confirmation dialog needs to able work nicely without an icon. Simple enough. Non-confirmation operator popups currently pass around a `wmOpPopUp` which contains only the following: ``` struct wmOpPopUp { wmOperator *op; int width; int height; int free_op; }; ``` We can just add these four items to the larger `wmConfirmDetails` (with width becoming min_width) and then rename `wmConfirmDetails` to `wmOpPopUp`. And then also add an `include_properties` bool. We should eventually end up with all dialogs that are much more configurable and sharing more common code.

image.png

304 KiB

image.png

19 KiB

image.png

25 KiB

Harley Acheson added the

Type

Design

label 2024-01-24 21:46:03 +01:00

Harley Acheson added this to the User Interface project 2024-01-24 21:46:05 +01:00

Harley Acheson commented

2024-01-24 21:54:34 +01:00

@brecht

This does include your comment to "change the new wm_block_confirm_create to call ot->ui between the message and the buttons, so that these new confirmation popups support showing operator properties" but makes this optional.

@mont29 - I think this is closer to what you are wanting, where I can use existing "ui" and op popup code for things like that "manage orphans" dialog. Make more sense to use dialog code rather than pretend it is a confirmation.

@brecht This does include [your comment](https://projects.blender.org/blender/blender/pulls/117242#issuecomment-1105918) to "change the new wm_block_confirm_create to call ot->ui between the message and the buttons, so that these new confirmation popups support showing operator properties" but makes this optional. @mont29 - I think this is closer to what you are wanting, where I can use existing "ui" and op popup code for things like that "manage orphans" dialog. Make more sense to use dialog code rather than pretend it is a confirmation.

Bastien Montagne commented

2024-01-25 10:20:39 +01:00

@Harley I think trying to reduce the amount of options here, and completely different codepaths doing seemingly (almost) the same thing, is definitely a good target.

On a high level, your proposal makes sense to me. I would even say we could entirely remove the WM_operator_props_dialog_popup once the WM_operator_confirm API is able to show the settings of an operator.

On a lower, technical level, I cannot say without deeper analysis of the existing code, whether the 'confirm' codepath should be merged into the 'props_dialog_popup' one, or vice-versa:

WM_operator_confirm family uses UI_popup_block_invoke to create and manage the popup window.
WM_operator_props_dialog_popup uses UI_popup_block to create and manage the popup window.

Maybe @ideasman42 or @JulianEisel know what are the differences between these, and which one should be used for operator popups?

@brecht

This does include your comment to "change the new wm_block_confirm_create to call ot->ui between the message and the buttons, so that these new confirmation popups support showing operator properties" but makes this optional.

Might be obvious, but technically, I don't think this code should ever directly call ot->ui, it should use uiTemplateOperatorPropoertyButs (as done in wm_block_dialog_create).

@Harley I think trying to reduce the amount of options here, and completely different codepaths doing seemingly (almost) the same thing, is definitely a good target. On a high level, your proposal makes sense to me. I would even say we could entirely remove the `WM_operator_props_dialog_popup` once the `WM_operator_confirm` API is able to show the settings of an operator. ------------ On a lower, technical level, I cannot say without deeper analysis of the existing code, whether the 'confirm' codepath should be merged into the 'props_dialog_popup' one, or vice-versa: * `WM_operator_confirm` family uses `UI_popup_block_invoke` to create and manage the popup window. * `WM_operator_props_dialog_popup` uses `UI_popup_block` to create and manage the popup window. Maybe @ideasman42 or @JulianEisel know what are the differences between these, and which one should be used for operator popups? ---------- > @brecht > > This does include [your comment](https://projects.blender.org/blender/blender/pulls/117242#issuecomment-1105918) to "change the new wm_block_confirm_create to call ot->ui between the message and the buttons, so that these new confirmation popups support showing operator properties" but makes this optional. Might be obvious, but technically, I don't think this code should ever directly call `ot->ui`, it should use `uiTemplateOperatorPropoertyButs` (as done in `wm_block_dialog_create`).

Brecht Van Lommel commented

2024-01-25 14:08:06 +01:00

In the Python API we currently have:

invoke_props_popup: show operator properties and execute it automatically on changes
invoke_props_dialog: show operator properties and only execute it on click of OK button
invoke_confirm: only to let user confirm the execution, no operator properties shown
invoke_popup: only shows operator's properties, without executing it
invoke_search_popup: searches values of an enum property of the operator

This set of API function seems quite sensible and corresponding to different use cases we need. And we should not break compatibility. But it is missing control over title, icon, message and buttons, as well using inconsistent styling.

I think it would make sense to extend those API functions to cover what we need. And then use the same mechanism and naming in C++ and Python. So all those functions would have the same name in C++, but with a WM_operator_ prefix. The new confirm callback mechanism that is only in C++ doesn't seem like a great fit and I would remove that in favor of an extended invoke_confirm.

I think title, icon and message could be added as optional arguments to these existing API functions.

I'd also probably just add optional text label for the confirmation and cancel buttons as well, along with a choice for which one should be the default. It does make the argument list relatively long. But introducing an intermediate struct doesn't see so convenient at least for the Python API. For C++ I don't mind a struct passed to these API functions if that makes the code cleaner.

A limitation is if you want to show e.g. 3 different buttons that correspond to different actions. Maybe that is best handled by invoke_popup which wouldn't automatically add any buttons. Then for the draw callback, there could be a new API function to add a dialog button with the right styling and callbacks.

For invoke_confirm, a design question is if we want to change to the new layout by default. The easiest incremental step would be to add a compact argument that defaults to True and keeps the existing behavior. Then it would be up to the UI module to decide if they want to turn that option off by default or not.

In the Python API we currently have: * `invoke_props_popup`: show operator properties and execute it automatically on changes * `invoke_props_dialog`: show operator properties and only execute it on click of OK button * `invoke_confirm`: only to let user confirm the execution, no operator properties shown * `invoke_popup`: only shows operator's properties, without executing it * `invoke_search_popup`: searches values of an enum property of the operator This set of API function seems quite sensible and corresponding to different use cases we need. And we should not break compatibility. But it is missing control over title, icon, message and buttons, as well using inconsistent styling. I think it would make sense to extend those API functions to cover what we need. And then use the same mechanism and naming in C++ and Python. So all those functions would have the same name in C++, but with a `WM_operator_` prefix. The new `confirm` callback mechanism that is only in C++ doesn't seem like a great fit and I would remove that in favor of an extended `invoke_confirm`. I think title, icon and message could be added as optional arguments to these existing API functions. I'd also probably just add optional text label for the confirmation and cancel buttons as well, along with a choice for which one should be the default. It does make the argument list relatively long. But introducing an intermediate struct doesn't see so convenient at least for the Python API. For C++ I don't mind a struct passed to these API functions if that makes the code cleaner. A limitation is if you want to show e.g. 3 different buttons that correspond to different actions. Maybe that is best handled by `invoke_popup` which wouldn't automatically add any buttons. Then for the `draw` callback, there could be a new API function to add a dialog button with the right styling and callbacks. For `invoke_confirm`, a design question is if we want to change to the new layout by default. The easiest incremental step would be to add a `compact` argument that defaults to `True` and keeps the existing behavior. Then it would be up to the UI module to decide if they want to turn that option off by default or not.

👍 2

Brecht Van Lommel commented

2024-01-25 14:09:17 +01:00

I imagine it's just for demonstration purposes, but I don't think that "Apply Object Transformations" popup should show the properties. If it wasn't going to show them when all the meshes were single user, it shouldn't show them if they aren't either. It should be purely a confirmation to make sure that is the main thing the users sees.

Harley Acheson commented

2024-01-25 17:16:10 +01:00

whether the 'confirm' codepath should be merged into the 'props_dialog_popup' one, or vice-versa

I'll try to treat anything in the props_dialog_popup version as correct over mine.

it should use uiTemplateOperatorPropoertyButs

Yes, I noticed that. I made a test implementation - #117493 - to see if it would work, and it uses that. Although don't look at that, I just wanted something to refer to later.

just for demonstration purposes, but I don't think that "Apply Object Transformations"

Yes, it was just handy - but useless.

the new confirm callback mechanism that is only in C++ doesn't seem like a great fit

Agreed. I think we can later add python access later, similar to shown here: #117143

I think the first steps are simple and non-contentious:

I'm going to remove some unneeded options from wmConfirmDetails. Some things were put in there just to see if the idea worked but are unnecessary. This would include the overwriting of the string of the "cancel" button. Not used now, but having it could lead to inconsistencies later where we get things like "yes/no" dialogs which are a bit evil.

I'll refactor wmConfirmDetails to use std::string instead of char arrays.

Then I'll make something for this consolidation that we can then look at.

> whether the 'confirm' codepath should be merged into the 'props_dialog_popup' one, or vice-versa I'll try to treat anything in the props_dialog_popup version as correct over mine. > it should use uiTemplateOperatorPropoertyButs Yes, I noticed that. I made a test implementation - #117493 - to see if it would work, and it uses that. Although don't look at that, I just wanted something to refer to later. > just for demonstration purposes, but I don't think that "Apply Object Transformations" Yes, it was just handy - but useless. > the new confirm callback mechanism that is only in C++ doesn't seem like a great fit Agreed. I think we can later add python access later, similar to shown here: #117143 I think the first steps are simple and non-contentious: I'm going to remove some unneeded options from wmConfirmDetails. Some things were put in there just to see if the idea worked but are unnecessary. This would include the overwriting of the string of the "cancel" button. Not used now, but having it could lead to inconsistencies later where we get things like "yes/no" dialogs which are a bit evil. I'll refactor wmConfirmDetails to use std::string instead of char arrays. Then I'll make something for this consolidation that we can then look at.

Harley Acheson referenced this issue

2024-03-01 00:06:42 +01:00

Roadmap #118940

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Consolidation of Operator Dialogs #117489