UI: Operator Confirm Dialog Changes #117564
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#117564
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "Harley/blender:NewConfirm"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Removal off "confirm" callback in favour of new method that shares
existing operator dialog code and allows python configuration.
For confirmations we are currently using a
WM_operator_confirm
function that is quite simple and can be used directly as ainvoke
operator callback. This results in a very simple popup, but we can create more complex versions by defining aconfirm
operator callback that allows overriding default values supplied as an argument. But there are downsides of this approach, the most important being the lack of python access.This PR creates a more complex
WM_operator_confirm_ex
instead. It can't be used directly as an invoke, but you instead call this within the operator invoke. This is a bit simpler and much more direct. It also fits better with future desires to have the confirmations enabled/disabled in userprefs rather than in the keymap. This PR removes theconfirm
callback.To help keep everything as consistent as possible, the dialog created is the same one created by the Operator Props dialog, just with different options. This means that
wm_block_confirm_create
is removed in this PR.The PR is just the creation of this
WM_operator_confirm_ex
function, a lot of boilerplate inrna_wm_api.cc
creating new properties for theinvoke_confirm
function. Some cleanup changing some enums to have names with wmPopup. And thenwm_block_dialog_create
gets some new abilities, like showing an alert icon, dynamic width based on string lengths (and using the passed "width" argument as a minimum), and improved positioning.There is an example of usage in this PR. In
wm_files.cc
you will seewm_clear_recent_files_exec
has very small change, with confirmation configuration moved to itsinvoke
.@guishe can you take a look?
@ -213,0 +214,4 @@
wmEvent * /*event*/,
const char *title,
const char *message,
const char *message2,
This shouldn't be necessary, especially not in the public API. The message can be split by newline characters?
@ -213,0 +220,4 @@
const int size,
const int position,
const bool cancel_default,
const bool mouse_move_quit,
I'm not sure about having
size
,position
andcancel_default
andmouse_move_quit
as parameters. Maybe evenicon
.It's leaving a lot of choice to each operator, which is not going to give consistency. I would prefer there rather to be an enum for different types of confirmation dialogs that we can style consistently.
At the moment I can only think of:
Does that make sense? Can you think of other types?
All good points. I had started this stuff a long time ago and had tried to keep my own opinions of their use out of the design. It was more about giving options to others who would decide on how they might use them. As in I envisioned someone else making decisions that used these options in a consistent way.
But my own opinion is a lot simpler.
Property dialogs are about perfect as they are now in main, with only ability to customize the title and confirm button text that should be a verb related to the operation. No additional content, ability to change the cancel text, and no icon.
For confirmations though I think we will only really need two types, and those can be determined automatically based on content.
If a confirmation can get by with a single line a text - just the "title" here, then we can assume that there isn't complication or nuance. "Delete this thing" does not require extra explanation so doesn't take much thought. Anything this simple can be small, just a small version of the icon (32x32 probably), that one line of text, and the two buttons at regular vertical size. Being small they can appear at mouse position. And I think these can all have the mouse move dismiss.
But as soon as we have to add more information, this means we really want the user to stop, read it, and think before proceeding. And I think those should all be the larger version - basically all the ones we have been evaluating lately like #117157. Those have a larger icon (64x64) and buttons that are a bit taller (1.2X). I think those should all appear at window center and should not do mouse move dismiss.
I really can't think any other types I'd want to see. So I would also remove the size, position, cancel_default, mouse_move_quit from the public API, but keep icon. I would remove these from
WM_operator_confirm_ex
as well, and just set these things based on whethermessage
text is added.I think as soon as we start talking about those smaller (and default) confirmations we'll be hit with the "actually we don't want those at all" and "that mouse move feature is bad" from some users, while others like them. Which is why I want to go to having these enabled/disabled in userprefs. At that point the confirmations can afford to get more content if they can use them, since they will be seen by those who want them. For examples of these I mean almost everything now configurable in the keymap, so normally using
WM_operator_confirm_or_exec
. Deleting keyframes, markers, bones, etc.One tangential thing is that once we see them in action from #117310 I think we'd want a horizontal rule between the title and the rest of the content for Property dialogs, the larger versions of the Confirmations, and probably popovers too.
Sorry about writing a novel...
Ok, can we then just add title, message, confirm text, icon and translations? And automatically using the bigger popup if these are customized.
The desired behavior for position and mouse move, and when to use them which style or use a popup at all, can then be decides by the UI module.
Yes, sounds like a plan. The only "iffy" part is that ability to make the cancel button the default. I was arguing its need but we only use it in one of our seven examples, "Load Factory Settings", and even that takes explicit action to get there; you don't get there accidentally. Would be easy to add back later if need be anyway though.
@brecht
Chickened out only slightly for
default_cancel
, removing it from the API but still having it as an optional argument forWM_operator_confirm_ex.
That way we can experiment with this setting for "Load Factory Settings."But otherwise the single Message text now handles newlines. Sizing, positioning, etc handled automatically based on the presence of the Message text. Without it we get a much smaller dialog that opens at mouse position and closes with mouse move.
Note (to other readers) that this PR does NOT change any of the default confirmations, for example the one we get when we press "x" for an mesh object. It is included in the following capture only to compare sizes. The following compares a current default confirm with a custom confirmation without message body, and one with message body:
@ -1717,1 +1545,3 @@
uiItemS_ex(layout, 0.3f);
/* Break Message into multiple lines. */
std::vector<std::string> message_lines;
std::istringstream origStream(data->message);
origStream
->message_stream
@ -1718,0 +1547,4 @@
std::istringstream origStream(data->message);
std::string line;
while (std::getline(origStream, line)) {
if (!line.empty()) {
Might as well keep empty lines if someone wants paragraphs.
You could trim whitespace at the start and end still:
@ -1718,0 +1558,4 @@
dialog_width += (data->icon == ALERT_ICON_NONE) ? 0 : icon_size;
/* Adjust width if the button text is long. */
int longest_button_text = std::max(
const int
@ -1718,0 +1560,4 @@
/* Adjust width if the button text is long. */
int longest_button_text = std::max(
BLF_width(style->widget.uifont_id, data->confirm_text.c_str(), data->confirm_text.length()),
BLF_width(style->widget.uifont_id, IFACE_("Cancel"), BLF_DRAW_STR_DUMMY_MAX));
Use
BLF_DRAW_STR_DUMMY_MAX
forBLF_width
calls?Then it will be a more obvious step to port the BLF API to C++ and make
BLF_DRAW_STR_DUMMY_MAX
the default or useStringRef
to simplify this.@ -1718,0 +1561,4 @@
int longest_button_text = std::max(
BLF_width(style->widget.uifont_id, data->confirm_text.c_str(), data->confirm_text.length()),
BLF_width(style->widget.uifont_id, IFACE_("Cancel"), BLF_DRAW_STR_DUMMY_MAX));
dialog_width = std::max(dialog_width, 3 * longest_button_text);
It's a bit unfortunate that all this logic is needed here. Maybe one day this can become a feature of the layout engine instead, but seems ok for now.
Actually we did have a
uiItemsAlertBox
routine in layout that's close, so made an overloaded variation that shares code with it.@ -1804,1 +1665,3 @@
UI_block_bounds_set_popup(block, 10 * UI_SCALE_FAC, bounds_offset);
bounds_offset[0] = uiLayoutGetWidth(layout) * (windows_layout ? -0.33f : -0.66f);
bounds_offset[1] = int(UI_UNIT_Y * (small ? 1.8 : 3.1));
UI_block_bounds_set_popup(block, padding, bounds_offset);
It wasn't immediately obvious to me what these magic values mean. Something like this would have helped:
How about instead of
default_cancel
something likeenter_confirms
?The popups already can be closed or canceled with
esc
button.So popups that are more dangerous just don't do anything on
enter
, only when clicking the confirm button.@ -1718,0 +1548,4 @@
std::string line;
while (std::getline(origStream, line)) {
if (!line.empty()) {
message_lines.push_back(line);
This could also break long lines to prevent popups from being too wide (like tooltips do)
Okay, I could be wrong about this, but I think that would require a feature that we are missing but need...
Tooltips do that wrapping within BLF. As in the FontBLF has a
BLF_WORD_WRAP
flag andwrap_width
member and when measuring the string or printing it out we run a callback that does wrapping.I don't think we have anything that helps with wrapping outside of text output. As in give it a string and fontid and get back info on where to break. Or breaks it up into a string vector. Or array of breaking points? That way we could then add labels to the layout for each substring, rather than printing out.
Haven't thought it through much, but I think such a thing could look like
blf_font_draw_buffer__wrap
but would pass an integer vector or array for the userdata and a callback that would populate it with wrapping points?Edit: This is probably a perfect use case for this. I'll try to get to this over this weekend.
I think
BLF_width_to_strlen
may be usefulYes, that gives the string byte index that fits within a pixel width. So you could iterate through the string looking for word delimiters and line breaks.
That is basically what the existing
blf_font_wrap_apply
does for us, and has a callback at each wrapping location, and can return the numbers of wrapped lines and the width of the last orphan line.UI: Operator Confirm Dialog Changesto WIP: UI: Operator Confirm Dialog ChangesWIP: UI: Operator Confirm Dialog Changesto UI: Operator Confirm Dialog Changes@brecht - (From #117528) - description is not meant for this...
Yes, and that applies to the confirmations as well. So I changed the defaults for them here to also use operator Name as title and "OK" for confirm button text. This means that calling
WM_operator_confirm_ex(C, op)
gives us something very close to the default confirms:@ -1367,4 +1216,0 @@
UI_block_func_set(block, nullptr, nullptr, nullptr);
UI_but_func_set(confirm_but, wm_operator_block_confirm, op, block);
UI_but_func_set(cancel_but, wm_operator_block_cancel, op, block);
Seems more code can be removed.
@ -1805,0 +1656,4 @@
const float button_center_x = windows_layout ? -0.33f : -0.66f;
const float button_center_y = small ? 1.9f : 3.1f;
const int bounds_offset[2] = {button_center_x * uiLayoutGetWidth(layout),
button_center_y * UI_UNIT_X};
Clang needs casts to compile this.
Did not mean to approve yet.
@blender-bot build