Re: [PATCH] comctl32/taskdialog: Implement buttons.

[Fabian Maurer]

Thanks for your comments, I fixed most of the issues you pointed out.
> > +static const UINT DIALOG_SPACING_BUTTON_H = 1; /* Distance between
> > buttons */ +static const UINT DIALOG_SPACING_BUTTON_W = 4; /* Distance
> > between buttons */
>
> It's tempting to use DIALOG_SPACING here as well to reduce amount of magic.

Did that for DIALOG_SPACING_BUTTON_W, but I'd keep DIALOG_SPACING_BUTTON_H, 
since the vertical distance between buttons on my windows machine is only 1px, 
5px sounds like a bit much in that case.
> This doesn't handle NULL text properly. At least buttons can't have text
> set to NULL

Added a check for that before trying to create the template.
> > - button.width = rect.right + 10;
>
> Why 10?

Should be dialog-spacing times two, missed that constant.
> > - /* 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 the
> >
> > right */ +                    buttons[j].x += diff;
> > -
> >            alignment = buttons[0].x; /* left-align all coming rows
> >
> > to 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;
> > - }
>
> This does not do the right thing. Having first row right-aligned is not
> a goal. According to my manual testing all rows are left aligned, and
> wrapped as soon as next button width exceeds dialog width, which by the
> way is supposed to grow, if cxWidth is 0. We can ignore that for now I
> guess.

Can't confirm that on my machine. The buttons are right aligned, and if there 
is more than one row, the second is left aligned to the first one.
For example, if you have one button, it's always on the right. If you have two 
rows of 4 buttons, it's aligned right, too.
While my code doesn't take into account all corner cases, like one button 
that's bugger than all others, it gets the main look done.

Regards,
Fabian Maurer