On 03.04.2017 13:53, Fabian Maurer wrote:
On Freitag, 24. März 2017 16:50:38 CEST Fabian Maurer wrote:
-static const UINT DIALOG_MIN_WIDTH = 180; +static const UINT DIALOG_MIN_WIDTH = 240;
static const UINT DIALOG_SPACING = 5;
-static const UINT DIALOG_BUTTON_WIDTH = 50; -static const UINT DIALOG_BUTTON_HEIGHT = 14; +static const UINT DIALOG_BUTTON_WIDTH_MIN = 45; +static const UINT DIALOG_BUTTON_HEIGHT = 13;
This was taken from MSGBOX template, what's a point in making it different?
I just tested it on my Win7 box and tried to get it to look as similar as possible. Where do the MSGBOX template values come from? I can remove that change, if those values are better.
Did you test that buttons are supposed to support multiple lines of text?
Normal buttons don't allow multiple lines of text. But we only use the right side of the rectangle anyways, so it works as expected. Problem is, on windows and on wine the buttons clips the text when it gets too long, so I'm not fully it's the correct way to implement this. Should I change this? There's also the fact that buttons get larger in height on windows, even if the text is a single line only. Should I reproduce this? Moreover, wine displays newlines as boxes while this isn't the case on windows. Not sure how to deal with this though.
static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc, WORD id, const WCHAR *class,>
HINSTANCE hInstance, const WCHAR *text, short x, short y, shortcx, short cy) + DWORD style, HINSTANCE hInstance, const WCHAR *text, short x, short y, short cx, short cy)>
{
struct taskdialog_control *control = Alloc(sizeof(*control)); unsigned int size, class_size, text_size;@@ -117,7 +167,7 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc>
control->template = template = Alloc(size); control->template_size = size;
- template->style = WS_VISIBLE;
- template->style = WS_VISIBLE | WS_CHILD | style;
You don't use styles in this patch, this is unnecessary change.
I'm using BS_PUSHBUTTON for the buttons, is this unnecessary?
template->dwExtendedStyle = 0; template->x = x; template->y = y;@@ -136,78 +186,126 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc>
static unsigned int taskdialog_add_main_instruction(struct taskdialog_template_desc *desc) {
- RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /*
padding left and right of the control */ - const WCHAR *textW = NULL;
- unsigned int size, length;
- HFONT oldfont;
- HDC hdc;
unsigned int size;
RECT rect;
if (!desc->taskconfig->pszMainInstruction)
return 0;
- if (IS_INTRESOURCE(desc->taskconfig->pszMainInstruction))
- {
if (!(length = LoadStringW(desc->taskconfig->hInstance,(UINT_PTR)desc->taskconfig->pszMainInstruction, - (WCHAR *)&textW, 0)))
{WARN("Failed to load main instruction text\n");return 0;}- }
- else
- {
textW = desc->taskconfig->pszMainInstruction;length = strlenW(textW);- }
- hdc = GetDC(0);
- oldfont = SelectObject(hdc, desc->font);
- dialogunits_to_pixels(desc, &rect.right, NULL);
- DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS |
DT_CALCRECT | DT_WORDBREAK); - pixels_to_dialogunits(desc, &rect.right, &rect.bottom);
- SelectObject(hdc, oldfont);
- ReleaseDC(0, hdc);
- rect = text_get_rect(desc, desc->taskconfig->pszMainInstruction);
- size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION,
WC_STATICW, desc->taskconfig->hInstance, + size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, 0, desc->taskconfig->hInstance,>
desc->taskconfig->pszMainInstruction, DIALOG_SPACING, desc->dialog_height, rect.right, rect.bottom);
- desc->dialog_height += rect.bottom;
desc->dialog_height += rect.bottom + DIALOG_SPACING;
return size;
}
-static unsigned int taskdialog_add_common_buttons(struct taskdialog_template_desc *desc) +static button_info make_button(struct taskdialog_template_desc *desc, int id, const WCHAR *text) +{
- RECT rect;
- button_info button;
- button.id = id;
- button.text = text;
- rect = text_get_rect(desc, text);
- button.width = rect.right + 10;
- if (button.width < DIALOG_BUTTON_WIDTH_MIN)
button.width = DIALOG_BUTTON_WIDTH_MIN;- return button;
+}
This does not handle "text" correctly.
Sorry, but I don't see the problem here. Do you mind explaining? In both text_get_rect and later on taskdialog_add_control both text or a resource-ID are allowed.
+static unsigned int taskdialog_add_buttons(struct taskdialog_template_desc *desc)>
{
- short button_x = desc->dialog_width - DIALOG_BUTTON_WIDTH -
DIALOG_SPACING;>
DWORD flags = desc->taskconfig->dwCommonButtons;
- const TASKDIALOGCONFIG *taskconfig = desc->taskconfig;
- UINT alignment = DIALOG_SPACING_BUTTONS_LEFT; /* minimum distance
from the left dialog border */ + UINT location_x;
BOOL first_row = TRUE;
button_info *buttons;
int count = 0;
int i;
unsigned int size = 0;
-#define TASKDIALOG_ADD_COMMON_BUTTON(id) \
- do { \
size += taskdialog_add_control(desc, ID##id, WC_BUTTONW,COMCTL32_hModule, MAKEINTRESOURCEW(IDS_BUTTON_##id), \ - button_x, desc->dialog_height + DIALOG_SPACING, DIALOG_BUTTON_WIDTH, DIALOG_BUTTON_HEIGHT); \ - button_x -= DIALOG_BUTTON_WIDTH + DIALOG_SPACING; \
- } while(0)
- if (flags & TDCBF_CLOSE_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(CLOSE);- if (flags & TDCBF_CANCEL_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(CANCEL);- if (flags & TDCBF_RETRY_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(RETRY);- if (flags & TDCBF_NO_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(NO);- if (flags & TDCBF_YES_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(YES);
- /* Allocate enough memory for the custom and the default buttons */
- if (taskconfig->cButtons && taskconfig->pButtons)
buttons = HeapAlloc(GetProcessHeap(), 0, (taskconfig->cButtons
- sizeof(button_info)); + else
- {
buttons = HeapAlloc(GetProcessHeap(), 0, 6 *sizeof(button_info));
- }
- /* Custom buttons */
- if (taskconfig->cButtons && taskconfig->pButtons)
- {
for (i = 0; i < taskconfig->cButtons; i++){buttons[count++] = make_button(desc,taskconfig->pButtons[i].nButtonID, taskconfig->pButtons[i].pszButtonText); + }
}
/* Default buttons */
if (flags & TDCBF_OK_BUTTON)
TASKDIALOG_ADD_COMMON_BUTTON(OK);- /* Always add OK button */
- if (list_empty(&desc->controls))
TASKDIALOG_ADD_COMMON_BUTTON(OK);-#undef TASKDIALOG_ADD_COMMON_BUTTON
- /* make room for common buttons row */
- desc->dialog_height += DIALOG_BUTTON_HEIGHT + 2 * DIALOG_SPACING;
buttons[count++] = make_button(desc, IDOK,MAKEINTRESOURCEW(IDS_BUTTON_OK)); + if (flags & TDCBF_YES_BUTTON)
buttons[count++] = make_button(desc, IDYES,MAKEINTRESOURCEW(IDS_BUTTON_YES)); + if (flags & TDCBF_NO_BUTTON)
buttons[count++] = make_button(desc, IDNO,MAKEINTRESOURCEW(IDS_BUTTON_NO)); + if (flags & TDCBF_RETRY_BUTTON)
buttons[count++] = make_button(desc, IDRETRY,MAKEINTRESOURCEW(IDS_BUTTON_RETRY)); + if (flags & TDCBF_CANCEL_BUTTON)
buttons[count++] = make_button(desc, IDCANCEL,MAKEINTRESOURCEW(IDS_BUTTON_CANCEL)); + if (flags & TDCBF_CLOSE_BUTTON)
buttons[count++] = make_button(desc, IDCLOSE,MAKEINTRESOURCEW(IDS_BUTTON_CLOSE)); +
- /* There must be at least one button */
- if (count == 0)
buttons[count++] = make_button(desc, IDOK,MAKEINTRESOURCEW(IDS_BUTTON_OK)); +
- /* Position buttons */
- location_x = alignment;
- for (i = 0; i < count; i++)
- {
/* When beginning new row, align the first */if (location_x + buttons[i].width + DIALOG_SPACING_BUTTON_W >desc->dialog_width) + {
if (first_row){int diff = desc->dialog_width - location_x;first_row = FALSE;for (int j = 0; j < i; j++) /* Align first row to theright */ + buttons[j].x += diff;
alignment = buttons[0].x; /* left-align all coming rowsto the first row */ + }
location_x = alignment;desc->dialog_height += DIALOG_BUTTON_HEIGHT +DIALOG_SPACING_BUTTON_H; + }
buttons[i].x = location_x;buttons[i].y = desc->dialog_height;location_x += buttons[i].width + DIALOG_SPACING_BUTTON_W;- }
- if (first_row) /* Always align first row to the right */
- {
int diff = desc->dialog_width - (buttons[count - 1].x +buttons[count - 1].width + DIALOG_SPACING_BUTTON_W); + for (int i = 0; i < count; i++)
buttons[i].x += diff;- }
I don't like this. Why do you need temporary array, and iterating twice seems excessive.
The problem is, a later button might influence the position of the first button. That's why I created them as temporary array and then change their position once I know how. I also could iterate the loop twice I guess, but that sounded a bit ugly, too. If you have a better idea to address this problem, I'd be happy to hear it.
- for (i = 0; i < count; i++)
- {
size += taskdialog_add_control(desc, buttons[i].id, WC_BUTTONW,BS_PUSHBUTTON, COMCTL32_hModule, + buttons[i].text, buttons[i].x, buttons[i].y, buttons[i].width, DIALOG_BUTTON_HEIGHT); + }
This won't work for custom buttons.
Right, I missed that, thanks for pointing it out.
- size += taskdialog_add_common_buttons(&desc);
- size += taskdialog_add_buttons(&desc);
I'd rather have it separated if possible. I'll do some testing myself, before commenting further.
I see. Just figured it would make sense to handle all buttons in one function, since they are aligned together - if no command-links are used. Is there a reason you want it separated?
Regards, Fabian Maurer
Any update on this?
In addition to issues I mentioned already, I don't think this works properly (or at all) for custom buttons. What will happen if custom button has common id, or several custom buttons share the same id?