Re[2]: [lmi] wxDialog::OnOK() removed from wx

[Vadim Zeitlin]

On Sun, 24 Sep 2006 23:51:15 +0000 Greg Chicares <address@hidden> wrote:

GC> > 1. in about_dialog.cpp, but there it seems to have nothing to do with
GC> >    wxID_OK button handler
GC> 
GC> Here, the "Read the GNU General Public License" button is mapped
GC> to wxID_OK, and the other button to wxID_CANCEL. Is there a
GC> less confusing way to accomplish the same thing?

 I don't think it makes much sense to use wxID_OK button id for a button
opening the licence dialog. Either a custom id or, maybe, wxID_ABOUT, would
be more appropriate IMHO. Using wxID_CANCEL for closing the dialog is more
understandable, especially as you had to do it with older wxWidgets
versions to ensure that the button is closed automatically when "Esc" key
is pressed but now you can use any id you like and just call wxDialog::
SetEscapeId() to indicate that the button with this id should be activated
when "Esc" is pressed. Also, wxID_OK is now mapped to "Esc" automatically
if there is no wxID_CANCEL.

 To summarize, I'd replace wxID_OK/CANCEL with wxID_ABOUT/OK or just used
custom LMI_ReadLicence and LMI_Close ids.

GC> > 2. in mvc_controller.cpp and there I think the same solution applies:
GC> >    it's better to move the code from UponOk() to TransferDataFromWindow()
GC> >    instead of conflating closing the dialog and validating data in it in
GC> >    the same handler. I can make a patch doing this if you'd like.
GC> 
GC> Here, UponOk()'s purpose is to close the dialog and display some
GC> information. But I really do think Validate() should be called
GC> here, to prevent closing the dialog when validation fails, just
GC> as UponPageChanging() vetoes a book-control page change in the
GC> same circumstance.

 This is exactly why I thought this should be in TransferDataFromWindow().
If this method returns false, the dialog is not closed.

GC> OTOH, 'lmi' does perform validation, but it does that not in a
GC> wxValidator override, but rather in a wxDialog override. We have
GC>   class MvcController : public wxDialog
GC> whose
GC>   virtual bool Validate();
GC> overrides wxDialog::Validate(), completely replacing it

 Validate() and TransferDataFromWindow() serve very similar purpose. I
usually prefer using the latter because I usually want to both validate and
save/transfer data and because of the symmetry with TransferDataToWindow()
but there is nothing wrong with using the former, of course.

GC> (I'll add a comment to that effect, because it doesn't call Skip());

 Err. it can't call Skip() because there are no events involved here.

GC> that function actually performs validation, and doesn't directly
GC> transfer data itself.
GC> 
GC> So I think it might be better just to do this:
GC> 
GC> > GC> (2) Write its body inline.

 Sorry, I must be missing something but I just don't see how do you arrive
at this conclusion. How does the fact that you use Validate() imply that
you must replace the event handler for wxID_OK button?

 Thanks,
VZ