Re: [PATCH 06/18] comctl32: TaskDialog - Implement main and content text controls

[Nikolay Sivov]

On 25.02.2017 19:01, Fabian Maurer wrote:
> +            PostMessageW(GetDlgItem(hwndDlg, ID_TEXTMAIN), WM_SETFONT, (WPARAM)font_main, TRUE);
> +            PostMessageW(GetDlgItem(hwndDlg, ID_TEXTCONTENT), WM_SETFONT, (WPARAM)font_default, TRUE);

Why Post*? Dialog handling in user32 has an ability to do this for you.
> +        case WM_CTLCOLORSTATIC:
> +               if((HWND)lParam == GetDlgItem(hwndDlg, ID_TEXTMAIN))
> +               {
> +                   HDC hdc = (HDC) wParam;
> +
> +                   SetTextColor(hdc, RGB(50,50,220));
> +                   SetBkColor(hdc, GetSysColor(COLOR_3DFACE));
> +
> +                   return (INT_PTR)GetSysColorBrush(COLOR_3DFACE);
> +               }
> +               break;

Where does this come from?
> +    font_default = CreateFontW (16, 0, 0, 0, FW_DONTCARE, FALSE, FALSE, FALSE,
> +                                DEFAULT_CHARSET, 0, 0, CLEARTYPE_QUALITY,  FF_DONTCARE, font_name);
> +    font_main = CreateFontW (19, 0, 0, 0, FW_DONTCARE, FALSE, FALSE, FALSE,
> +                                DEFAULT_CHARSET, 0, 0, CLEARTYPE_QUALITY,  FF_DONTCARE, font_name);
> +

Why Tahoma is hardcoded? If you're using dialog code this should be
handled automatically using template data.
> +    if(!IS_INTRESOURCE(pTaskConfig->pszMainInstruction) && !STR_EMPTY(pTaskConfig->pszMainInstruction))
> +    {
> +        RECT rect;
> +
> +        SelectObject(dc_dummy, font_main);
> +        rect = text_get_rect(dc_dummy, pTaskConfig->pszMainInstruction, dialog_width);
> +
> +        controls_add(controls, ID_TEXTMAIN, class_static, pTaskConfig->pszMainInstruction,
> +                     WS_CHILD | WS_VISIBLE, 5, dialog_height + 5, rect.right, rect.bottom);
> +
> +        dialog_height += rect.bottom + 10;
> +        SelectObject(dc_dummy, font_default);
> +    }
> +
> +    if(!IS_INTRESOURCE(pTaskConfig->pszContent) && !STR_EMPTY(pTaskConfig->pszContent))
> +    {
> +        RECT rect = text_get_rect(dc_dummy, pTaskConfig->pszContent, dialog_width);
> +
> +        controls_add(controls, ID_TEXTCONTENT, class_static, pTaskConfig->pszContent,
> +                     WS_CHILD | WS_VISIBLE, 5, dialog_height + 5, rect.right, rect.bottom);
> +
> +        dialog_height += rect.bottom + 10;
> +    }
> +
> +    controls_add(controls, IDOK, class_button, text_ok,
> +                 WS_CHILD | WS_VISIBLE | BS_DEFPUSHBUTTON, dialog_width - 40 - 10, dialog_height + 5, 40, 10);
> +    dialog_height += 10 + 10;

There's a lot of magic offsets here that should be grouped together and
probably have some defined names too.

[PATCH 06/18] comctl32: TaskDialog - Implement main and content text controls

[Fabian Maurer]

> Why Post*? Dialog handling in user32 has an ability to do this for you.
> Ah yes, I forgot about SendDlgItemMessage there.
> Where does this come from?
> What exactly, the Color? I just tried to pick one that is as similar to the 
> original taskdialog.
> Why Tahoma is hardcoded? If you're using dialog code this should be
> handled automatically using template data.
> How do you mean? I could add a font into the DLGTEMPLATE, but I need different 
> fonts for different controls. And since the user isn't allowed to choose the 
> font for a taskdialog (AFAIK), I just picked one.
> There's a lot of magic offsets here that should be grouped together and
> probably have some defined names too.
> Ok, I'll change that.

Regards,
Fabian Maurer

[PATCH 06/18] comctl32: TaskDialog - Implement main and content text controls

[Nikolay Sivov]

On 26.02.2017 1:32, Fabian Maurer wrote:
> > Why Post*? Dialog handling in user32 has an ability to do this for you.
> > Ah yes, I forgot about SendDlgItemMessage there.

What I mean is that dialog code can set control font for you.
> > Where does this come from?
> > What exactly, the Color? I just tried to pick one that is as similar to the 
> > original taskdialog.
> > Why Tahoma is hardcoded? If you're using dialog code this should be
> > handled automatically using template data.
> > How do you mean? I could add a font into the DLGTEMPLATE, but I need different 
> > fonts for different controls. And since the user isn't allowed to choose the 
> > font for a taskdialog (AFAIK), I just picked one.

Different how? In sizes only? Font name should come from
NONCLIENTMETRICS probably, this needs some manual testing, but it's
unlikely that system settings are not respected for task dialogs on Windows.

[PATCH 06/18] comctl32: TaskDialog - Implement main and content text controls

[Fabian Maurer]

> > > Why Post*? Dialog handling in user32 has an ability to do this for you.
> >
> > Ah yes, I forgot about SendDlgItemMessage there.
>
> What I mean is that dialog code can set control font for you.

How? I'm currently not aware of such a method. You mean DS_SETFONT and a font 
in the DLGTEMPLATE? But then we'd still have the different font sizes to 
handle.
> Different how? In sizes only? Font name should come from
> NONCLIENTMETRICS probably, this needs some manual testing, but it's
> unlikely that system settings are not respected for task dialogs on Windows.

Yes, it's only the font size that's different.

How would you suggest we handle the fonts?

Regards,
Fabian Maurer