[wx-dev] Re: CVS: [ABX] wxWidgets/src/generic aboutdlgg.cpp, 1.7, 1.8

[ABX]

Vadim Zeitlin <vadim at wxwindows.org>:
>> Update of /pack/cvsroots/wxwidgets/wxWidgets/src/generic
>> In directory sunsite.dk:/tmp/cvs-serv24513
>> 
>> Modified Files:
>> 	aboutdlgg.cpp 
>> Log Message:
>> Use CreateButtonSizer in wxAboutBox() to make OK button look native.
>
> I scratched my head a bit about this but then I realized that you probably
> meant "natively aligned", right? Thanks for fixing this!

Not really. On smartphone this is not button at all and this is converted to
hardware button action so there is not alignment at all. But, well, you know,
I'm not native speaker and will never ask for wxBlog account ;)

>> -    sizerTop->Add(new wxButton(this, wxID_OK), wxSizerFlags().Right().Border());
>> +
>> +    int defBorder = wxSizerFlags().Border().GetBorderInPixels();
>
> This is ugly though. I'd rather default CreateButtonSizer() border
>parameter to the same border wxSizerFlags uses. Or add
>wxSizer::GetDefaultBorder() static function (best probably). But creating
>wxSizerFlags just to get its border looks really strange.

Feel free. I just wanted to get Smartphone build working for time of release.

> > +    wxSizer *buttonSizer = CreateButtonSizer( wxOK , false, defBorder );
> > +    if(buttonSizer->GetChildren().GetCount() > 0 )
>
> I wonder what's wrong with buttonSizer->GetChildren().empty()... Also I
> think it's far from obvious why do we test for it at all, I think a comment
> along the lines of "some platforms don't use Ok button at all e.g. because
> they have hardware buttons" would be really helpful.

This pattern used in all generic dialogs. It is for Smartphones where there is
none component returned. Perhaps the comment could be added directly into
http://www.lpthe.jussieu.fr/~zeitlin/wxWindows/docs/wxwin_wxdialog.html#wxdialogcreatebuttonsizer
but again I'm poor in writing explanations.

>> +    {
>> +        sizerTop->Add( buttonSizer, 0, wxEXPAND | wxALL, defBorder );
>> +    }
>
> And this is inconsistent with the code just above :-( Why not
>
>	sizerTop->Add(buttonSizer, wxSizerFlags().Expand().Border())
>
> ?
> > +    else
> > +    {
> > +        sizerTop->AddSpacer( defBorder );
> > +        delete buttonSizer;
> > +    }
>
> Why do we need the spacer?

Good questions. I just followed patter used in generic dialogs. Even if I
participated in making them I don't remember explanation currently. I will try
to refresh memory once all the test builds will be completed. Thanks for
pointing them out.

ABX