UI: Operator Confirm Dialog Changes #117564
@ -214,31 +214,15 @@ static int rna_Operator_confirm(bContext *C,
|
||||
wmEvent * /*event*/,
|
||||
const char *title,
|
||||
const char *message,
|
||||
const char *message2,
|
||||
const char *confirm_text,
|
||||
Harley marked this conversation as resolved
|
||||
const int icon,
|
||||
const int size,
|
||||
const int position,
|
||||
const bool cancel_default,
|
||||
const bool mouse_move_quit,
|
||||
const char *text_ctxt,
|
||||
const bool translate)
|
||||
{
|
||||
title = RNA_translate_ui_text(title, text_ctxt, nullptr, nullptr, translate);
|
||||
message = RNA_translate_ui_text(message, text_ctxt, nullptr, nullptr, translate);
|
||||
brecht marked this conversation as resolved
Brecht Van Lommel
commented
I'm not sure about having 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? I'm not sure about having `size`, `position` and `cancel_default` and `mouse_move_quit` as parameters. Maybe even `icon`.
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:
* "quick" confirmation under the mouse cursor, relatively small, dismissed with mousemove, question icon.
* "important" confirmation, center of the window, relatively wide, no quit on mouse move, warning icon.
Does that make sense? Can you think of other types?
|
||||
message2 = RNA_translate_ui_text(message2, text_ctxt, nullptr, nullptr, translate);
|
||||
confirm_text = RNA_translate_ui_text(confirm_text, text_ctxt, nullptr, nullptr, translate);
|
||||
return WM_operator_confirm_ex(C,
|
||||
op,
|
||||
title,
|
||||
message,
|
||||
message2,
|
||||
confirm_text,
|
||||
icon,
|
||||
wmPopupSize(size),
|
||||
wmPopupPosition(position),
|
||||
cancel_default,
|
||||
mouse_move_quit);
|
||||
return WM_operator_confirm_ex(C, op, title, message, confirm_text, icon);
|
||||
}
|
||||
|
||||
static int rna_Operator_props_popup(bContext *C, wmOperator *op, wmEvent *event)
|
||||
@ -826,17 +810,6 @@ const EnumPropertyItem rna_operator_popup_icon_items[] = {
|
||||
{0, nullptr, 0, nullptr, nullptr},
|
||||
};
|
||||
|
||||
const EnumPropertyItem rna_operator_popup_size_items[] = {
|
||||
{WM_POPUP_POSITION_MOUSE, "MOUSE", 0, "Mouse", "Pop up at mouse position"},
|
||||
{WM_POPUP_POSITION_CENTER, "CENTER", 0, "Large", "Pop up at window center"},
|
||||
{0, nullptr, 0, nullptr, nullptr},
|
||||
};
|
||||
|
||||
const EnumPropertyItem rna_operator_popup_position_items[] = {
|
||||
{WM_CURSOR_DEFAULT, "DEFAULT", 0, "Default", ""},
|
||||
{0, nullptr, 0, nullptr, nullptr},
|
||||
};
|
||||
|
||||
void RNA_api_wm(StructRNA *srna)
|
||||
{
|
||||
FunctionRNA *func;
|
||||
@ -966,9 +939,6 @@ void RNA_api_wm(StructRNA *srna)
|
||||
parm = RNA_def_property(func, "message", PROP_STRING, PROP_NONE);
|
||||
RNA_def_property_ui_text(parm, "Message", "Optional first line of content text");
|
||||
|
||||
parm = RNA_def_property(func, "message2", PROP_STRING, PROP_NONE);
|
||||
RNA_def_property_ui_text(parm, "Message2", "Optional second line of content text");
|
||||
|
||||
parm = RNA_def_property(func, "confirm_text", PROP_STRING, PROP_NONE);
|
||||
RNA_def_property_ui_text(
|
||||
parm,
|
||||
@ -979,22 +949,6 @@ void RNA_api_wm(StructRNA *srna)
|
||||
RNA_def_property_enum_items(parm, rna_operator_popup_icon_items);
|
||||
RNA_def_property_ui_text(parm, "Icon", "Optional icon displayed in the dialog");
|
||||
|
||||
parm = RNA_def_property(func, "size", PROP_ENUM, PROP_NONE);
|
||||
RNA_def_property_enum_items(parm, rna_operator_popup_size_items);
|
||||
RNA_def_property_ui_text(parm, "Size", "Size of the popup");
|
||||
|
||||
parm = RNA_def_property(func, "position", PROP_ENUM, PROP_NONE);
|
||||
RNA_def_property_enum_items(parm, rna_operator_popup_position_items);
|
||||
RNA_def_property_ui_text(parm, "Position", "Position of the popup");
|
||||
|
||||
parm = RNA_def_property(func, "cancel_default", PROP_BOOLEAN, PROP_NONE);
|
||||
RNA_def_property_ui_text(
|
||||
parm, "Cancel Default", "Set to true to make the Cancel button the default action");
|
||||
|
||||
parm = RNA_def_property(func, "mouse_move_quit", PROP_BOOLEAN, PROP_NONE);
|
||||
RNA_def_property_ui_text(
|
||||
parm, "Mouse Move Quit", "Set to true close the popup by moving your mouse out of range");
|
||||
|
||||
api_ui_item_common_translation(func);
|
||||
|
||||
/* wrap UI_popup_menu_begin */
|
||||
|
@ -693,13 +693,9 @@ int WM_operator_confirm_ex(bContext *C,
|
||||
wmOperator *op,
|
||||
const char *title = nullptr,
|
||||
const char *message = nullptr,
|
||||
const char *message2 = nullptr,
|
||||
const char *confirm_text = nullptr,
|
||||
int icon = 0, /* ALERT_ICON_WARNING. */
|
||||
wmPopupSize size = WM_POPUP_SIZE_SMALL,
|
||||
wmPopupPosition position = WM_POPUP_POSITION_MOUSE,
|
||||
bool cancel_default = false,
|
||||
bool mouse_move_quit = false);
|
||||
bool cancel_default = false);
|
||||
|
||||
/**
|
||||
* Invoke callback, file selector "filepath" unset + exec.
|
||||
|
@ -929,18 +929,6 @@ enum wmPopupPosition {
|
||||
WM_POPUP_POSITION_CENTER,
|
||||
};
|
||||
|
||||
struct wmConfirmDetails {
|
||||
std::string title;
|
||||
std::string message;
|
||||
std::string message2;
|
||||
std::string confirm_text;
|
||||
int icon;
|
||||
wmPopupSize size;
|
||||
wmPopupPosition position;
|
||||
bool cancel_default;
|
||||
bool mouse_move_quit;
|
||||
};
|
||||
|
||||
/**
|
||||
* Communication/status data owned by the wmJob, and passed to the worker code when calling
|
||||
* `startjob` callback.
|
||||
|
@ -3586,12 +3586,8 @@ static int wm_clear_recent_files_invoke(bContext *C, wmOperator *op, const wmEve
|
||||
op,
|
||||
nullptr,
|
||||
IFACE_("Remove all items from the recent files list"),
|
||||
nullptr,
|
||||
IFACE_("Remove All"),
|
||||
ALERT_ICON_WARNING,
|
||||
WM_POPUP_SIZE_LARGE,
|
||||
WM_POPUP_POSITION_CENTER,
|
||||
false,
|
||||
false);
|
||||
}
|
||||
|
||||
|
@ -16,6 +16,8 @@
|
||||||||
#include <cstddef>
|
||||||||
#include <cstdio>
|
||||||||
#include <cstring>
|
||||||||
#include <iostream>
|
||||||||
#include <sstream>
|
||||||||
|
||||||||
#ifdef WIN32
|
||||||||
# include "GHOST_C-api.h"
|
||||||||
@ -1466,7 +1468,6 @@ struct wmOpPopUp {
|
||||||||
int free_op;
|
||||||||
std::string title;
|
||||||||
std::string message;
|
||||||||
std::string message2;
|
||||||||
std::string confirm_text;
|
||||||||
int icon;
|
||||||||
wmPopupSize size;
|
||||||||
@ -1520,6 +1521,8 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
wmOpPopUp *data = static_cast<wmOpPopUp *>(user_data);
|
||||||||
wmOperator *op = data->op;
|
||||||||
const uiStyle *style = UI_style_get_dpi();
|
||||||||
const bool small = data->size == WM_POPUP_SIZE_SMALL;
|
||||||||
const short icon_size = (small ? 32 : 64) * UI_SCALE_FAC;
|
||||||||
|
||||||||
uiBlock *block = UI_block_begin(C, region, __func__, UI_EMBOSS);
|
||||||||
UI_block_flag_disable(block, UI_BLOCK_LOOP);
|
||||||||
@ -1534,34 +1537,34 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
|
||||||||
UI_block_flag_enable(block, UI_BLOCK_KEEP_OPEN | UI_BLOCK_NUMSELECT);
|
||||||||
|
||||||||
/* Width based on the text lengths. */
|
||||||||
int text_width = std::max(
|
||||||||
120 * UI_SCALE_FAC,
|
||||||||
BLF_width(style->widget.uifont_id, data->title.c_str(), data->title.length()));
|
||||||||
if (!data->message.empty()) {
|
||||||||
text_width = std::max(
|
||||||||
text_width,
|
||||||||
int(BLF_width(style->widget.uifont_id, data->message.c_str(), data->message.length())));
|
||||||||
|
||||||||
/* Break Message into multiple lines. */
|
||||||||
std::vector<std::string> message_lines;
|
||||||||
std::istringstream origStream(data->message);
|
||||||||
Harley marked this conversation as resolved
Outdated
Brecht Van Lommel
commented
`origStream` -> `message_stream`
|
||||||||
std::string line;
|
||||||||
while (std::getline(origStream, line)) {
|
||||||||
if (!line.empty()) {
|
||||||||
Harley marked this conversation as resolved
Outdated
Brecht Van Lommel
commented
Might as well keep empty lines if someone wants paragraphs. You could trim whitespace at the start and end still:
Might as well keep empty lines if someone wants paragraphs.
You could trim whitespace at the start and end still:
```
StringRef messaged_trimmed = StringRef(data->message).trim();
std::istringstream message_stream(messaged_trimmed);
...
```
|
||||||||
message_lines.push_back(line);
|
||||||||
Guillermo Venegas
commented
This could also break long lines to prevent popups from being too wide (like tooltips do)
This could also break long lines to prevent popups from being too wide (like tooltips do)
| Long tool-tip message | Long confirm message |
| -------- | -------- |
|![image](/attachments/4192bc32-4111-4cdd-a6ac-3aeefdcda686)|![image](/attachments/0e82b141-f935-40a5-8462-82869c16d780)|
Harley Acheson
commented
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 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 Edit: This is probably a perfect use case for this. I'll try to get to this over this weekend. > @guishe - 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 and `wrap_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.
Guillermo Venegas
commented
I think I think `BLF_width_to_strlen` may be useful
Harley Acheson
commented
Yes, 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 > I think BLF_width_to_strlen may be useful
Yes, 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.
|
||||||||
text_width = std::max(text_width,
|
||||||||
int(BLF_width(style->widget.uifont_id, line.c_str(), line.length())));
|
||||||||
}
|
||||||||
if (!data->message2.empty()) {
|
||||||||
text_width = std::max(
|
||||||||
text_width,
|
||||||||
int(BLF_width(style->widget.uifont_id, data->message2.c_str(), data->message2.length())));
|
||||||||
}
|
||||||||
|
||||||||
const bool small = data->size == WM_POPUP_SIZE_SMALL;
|
||||||||
const short icon_size = (small ? (data->message.empty() ? 32 : 48) : 64) * UI_SCALE_FAC;
|
||||||||
int dialog_width = text_width + (style->columnspace * 2.5);
|
||||||||
int dialog_width = std::max(text_width + int(style->columnspace * 2.5), data->width);
|
||||||||
dialog_width += (data->icon == ALERT_ICON_NONE) ? 0 : icon_size;
|
||||||||
|
||||||||
uiLayout *layout = UI_block_layout(block,
|
||||||||
UI_LAYOUT_VERTICAL,
|
||||||||
UI_LAYOUT_PANEL,
|
||||||||
0,
|
||||||||
0,
|
||||||||
std::max(dialog_width, data->width),
|
||||||||
0,
|
||||||||
0,
|
||||||||
style);
|
||||||||
/* Adjust width if the button text is long. */
|
||||||||
int longest_button_text = std::max(
|
||||||||
Harley marked this conversation as resolved
Outdated
Brecht Van Lommel
commented
`const int`
|
||||||||
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));
|
||||||||
Harley marked this conversation as resolved
Outdated
Brecht Van Lommel
commented
Use Then it will be a more obvious step to port the BLF API to C++ and make Use `BLF_DRAW_STR_DUMMY_MAX` for `BLF_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 use `StringRef` to simplify this.
|
||||||||
dialog_width = std::max(dialog_width, 3 * longest_button_text);
|
||||||||
Brecht Van Lommel
commented
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. 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.
Harley Acheson
commented
Actually we did have a Actually we did have a `uiItemsAlertBox` routine in layout that's close, so made an overloaded variation that shares code with it.
|
||||||||
|
||||||||
uiLayout *layout = UI_block_layout(
|
||||||||
block, UI_LAYOUT_VERTICAL, UI_LAYOUT_PANEL, 0, 0, dialog_width, 0, 0, style);
|
||||||||
|
||||||||
if (data->icon != ALERT_ICON_NONE) {
|
||||||||
/* Split layout to put alert icon on left side. */
|
||||||||
@ -1576,18 +1579,14 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
layout = uiLayoutColumn(split_block, true);
|
||||||||
}
|
||||||||
|
||||||||
/* Title. */
|
||||||||
if (!data->title.empty()) {
|
||||||||
if (data->message.empty() && !small) {
|
||||||||
uiItemS(layout);
|
||||||||
}
|
||||||||
uiItemL_ex(layout, data->title.c_str(), ICON_NONE, true, false);
|
||||||||
}
|
||||||||
if (!data->message.empty()) {
|
||||||||
uiItemL(layout, data->message.c_str(), ICON_NONE);
|
||||||||
}
|
||||||||
|
||||||||
if (!data->message2.empty()) {
|
||||||||
uiItemL(layout, data->message2.c_str(), ICON_NONE);
|
||||||||
/* Message lines. */
|
||||||||
for (auto &st : message_lines) {
|
||||||||
uiItemL(layout, st.c_str(), ICON_NONE);
|
||||||||
}
|
||||||||
|
||||||||
if (data->include_properties) {
|
||||||||
@ -1613,7 +1612,7 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
uiBut *cancel_but;
|
||||||||
|
||||||||
col = uiLayoutSplit(col, 0.0f, true);
|
||||||||
uiLayoutSetScaleY(col, small ? 1.1f : 1.2f);
|
||||||||
uiLayoutSetScaleY(col, small ? 1.0f : 1.2f);
|
||||||||
|
||||||||
if (windows_layout) {
|
||||||||
confirm_but = uiDefBut(col_block,
|
||||||||
@ -1621,7 +1620,7 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
0,
|
||||||||
data->confirm_text.c_str(),
|
||||||||
0,
|
||||||||
-30,
|
||||||||
0,
|
||||||||
0,
|
||||||||
UI_UNIT_Y,
|
||||||||
nullptr,
|
||||||||
@ -1633,20 +1632,8 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
uiLayoutColumn(col, false);
|
||||||||
}
|
||||||||
|
||||||||
cancel_but = uiDefBut(col_block,
|
||||||||
UI_BTYPE_BUT,
|
||||||||
0,
|
||||||||
IFACE_("Cancel"),
|
||||||||
0,
|
||||||||
-30,
|
||||||||
0,
|
||||||||
UI_UNIT_Y,
|
||||||||
nullptr,
|
||||||||
0,
|
||||||||
0,
|
||||||||
0,
|
||||||||
0,
|
||||||||
"");
|
||||||||
cancel_but = uiDefBut(
|
||||||||
col_block, UI_BTYPE_BUT, 0, IFACE_("Cancel"), 0, 0, 0, UI_UNIT_Y, nullptr, 0, 0, 0, 0, "");
|
||||||||
|
||||||||
if (!windows_layout) {
|
||||||||
uiLayoutColumn(col, false);
|
||||||||
@ -1655,7 +1642,7 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
0,
|
||||||||
data->confirm_text.c_str(),
|
||||||||
0,
|
||||||||
-30,
|
||||||||
0,
|
||||||||
0,
|
||||||||
UI_UNIT_Y,
|
||||||||
nullptr,
|
||||||||
@ -1676,7 +1663,7 @@ static uiBlock *wm_block_dialog_create(bContext *C, ARegion *region, void *user_
|
||||||||
if (data->position == WM_POPUP_POSITION_MOUSE) {
|
||||||||
int bounds_offset[2];
|
||||||||
bounds_offset[0] = uiLayoutGetWidth(layout) * (windows_layout ? -0.33f : -0.66f);
|
||||||||
bounds_offset[1] = UI_UNIT_Y * (!data->message.empty() ? 3.1 : 2.5);
|
||||||||
bounds_offset[1] = int(UI_UNIT_Y * (small ? 1.8 : 3.1));
|
||||||||
UI_block_bounds_set_popup(block, padding, bounds_offset);
|
||||||||
Harley marked this conversation as resolved
Outdated
Brecht Van Lommel
commented
It wasn't immediately obvious to me what these magic values mean. Something like this would have helped:
It wasn't immediately obvious to me what these magic values mean. Something like this would have helped:
```
const float button_center_x = windows_layout ? -0.33f : -0.66f;
const float button_center_y = small ? 1.8f : 3.1f;
const int bounds_offset[2] = {button_center_x * uiLayoutGetWidth(layout),
button_center_y * UI_UNIT_X};
```
|
||||||||
}
|
||||||||
else if (data->position == WM_POPUP_POSITION_CENTER) {
|
||||||||
@ -1744,29 +1731,24 @@ int WM_operator_confirm_ex(bContext *C,
|
||||||||
wmOperator *op,
|
||||||||
const char *title,
|
||||||||
const char *message,
|
||||||||
const char *message2,
|
||||||||
const char *confirm_text,
|
||||||||
int icon,
|
||||||||
wmPopupSize size,
|
||||||||
wmPopupPosition position,
|
||||||||
bool cancel_default,
|
||||||||
bool mouse_move_quit)
|
||||||||
bool cancel_default)
|
||||||||
{
|
||||||||
wmOpPopUp *data = MEM_new<wmOpPopUp>(__func__);
|
||||||||
data->op = op;
|
||||||||
data->width = 400;
|
||||||||
data->width = 180 * UI_SCALE_FAC;
|
||||||||
data->free_op = true;
|
||||||||
data->title = (title == nullptr) ? WM_operatortype_description(C, op->type, op->ptr) : title;
|
||||||||
data->message = (message == nullptr) ? std::string() : message;
|
||||||||
data->message2 = (message2 == nullptr) ? std::string() : message2;
|
||||||||
data->confirm_text = (confirm_text == nullptr) ? WM_operatortype_name(op->type, op->ptr) :
|
||||||||
confirm_text;
|
||||||||
data->icon = icon;
|
||||||||
data->cancel_default = cancel_default;
|
||||||||
data->mouse_move_quit = mouse_move_quit;
|
||||||||
data->include_properties = false;
|
||||||||
data->size = size;
|
||||||||
data->position = position;
|
||||||||
data->cancel_default = cancel_default;
|
||||||||
data->mouse_move_quit = (message == nullptr) ? true : false;
|
||||||||
data->size = (message == nullptr) ? WM_POPUP_SIZE_SMALL : WM_POPUP_SIZE_LARGE;
|
||||||||
data->position = (message == nullptr) ? WM_POPUP_POSITION_MOUSE : WM_POPUP_POSITION_CENTER;
|
||||||||
|
||||||||
UI_popup_block_ex(
|
||||||||
C, wm_block_dialog_create, wm_operator_ui_popup_ok, wm_operator_ui_popup_cancel, data, op);
|
||||||||
|
This shouldn't be necessary, especially not in the public API. The message can be split by newline characters?