Re: Re: CvsAlert changes

Jens,

>Just one modification I'd suggest: make CvsAlert::ShowAlert a const
>member function. That would allow streamlined code like e.g.
>      CvsAlert("Some message").ShowAlert();
>for basic error messages.

Why do we need consts?
That works for me:
CvsAlert(kCvsAlertNoteIcon, "Some message").ShowAlert();

Unless you mean to skip the icon parameter, in which case we could have an
overloaded constructor. But I have the feeling that it's good to be forced
to specify the message type (icon) as it makes you think what you doing ;)

>And another thing I noticed: there's a const std::string m_empty
>member in the CvsAlert class, but IMO, this could be a static
>member and shared between all instances of CvsAlert?

Actually I decided to remove it all together and return the copies of
members instead of const references. It's the alert dialog after all and
there is no reason to optimize it. It's better to keep it simple ;)

I've commited that change - it will not affect the class usage and it will
definately eliminate confusion.

>I saw that e.g. for the alert asking to confirm moving files to
>the trash, you specified BUTTONTITLE_YES and
>BUTTONTITLE_NO as the button titles.
>However, on the Mac, questions of this kind (i.e. confirming
>operations) are not asked with "Yes" and "No" buttons, but rather with
>"OK" and "Cancel" buttons. (Of course this requires that the wording of
>the question itself is such that it's clear what OK and Cancel
>means...)

We have to go about it on a case-by-case basis. In the case of erasing files
in CvsCmdMoveToTrash the wording seems to match the Yes/No buttons rather
than OK/Cancel. On top of that the standard "Move to recycle bin" on Windows
uses the Yes/No buttons. But if you feel we have to customize that or any
other part then feel free to wrap the place with #ifdef's as needed. I think
it's better to have an external conditional compilation instead of the
abstract concept of default buttons. It's simpler to avoid too many
indirection. Eventually you could use different defaults in CvsAlert
contructor but it has the OK/Cancel as defaults already.

>My suggestion now is to allow CvsAlert to be called asynchronously by
>implementing a member ShowAlertAsync where a completion callback can be
>passed (preferably a functor) which will be called with the alert
>result when the alert is dismissed:

I think it's easier to make the CvsAlert::ShowAlert virtual and derive a
class from CvsAlert (like CvsAlertAsync) and then extend the derived class
constructor to pass the callback function and override ShowAlert to handle
asynchronous behaviour.

I would not like to have too many functions to show the alert - it may get
very confusing with time ;)

Best Regards,
Jerzy

------------------------ Yahoo! Groups Sponsor ---------------------~-->
Buy Ink Cartridges or Refill Kits for your HP, Epson, Canon or Lexmark
Printer at MyInks.com. Free s/h on orders $50 or more to the US & Canada.
http://www.c1tracking.com/l.asp?cid=5511
http://us.click.yahoo.com/mOAaAA/3exGAA/qnsNAA/NhFolB/TM
---------------------------------------------------------------------~->

To unsubscribe from this group, send an email to:
[email protected]

 

Your use of Yahoo! Groups is subject to http://docs.yahoo.com/info/terms/