Re: [PATCH] Refactor CBalloon code

Alexander Klenin wrote:
> * Removed repeating comments about compilation on NT4 & Win95,
> since these platforms are no longer supported
> * Simplified USE_GDI_GRADIENT usage with additional macro
> * Common subexpressions extracted is a few functions
> * Removed unused parameters and local variables
> * Fixed incorrect behavior of tooltip close button when user presses mouse
> button over it, but releases away from it, or vice versa.
> * Converted ShowBalloon functions to void, since their return value
> was not safe to use.

Thanks!
Committed in revision 10509.

> These are changes I made while trying to understand the logic and
> structure of CBalloon class in order to try implementing
> BALLOON_BORDER_ANCHOR style we discussed earlier.
>
> There are still things I not fully clear to me:
>
> 1) Is there a useful case when m_pParentWnd == NULL?
> If not, the assertion can be added to Create() function and the rest
> of the code simplified.

Yes, every time there isn't a parent dialog/window available and we'd
like to show a balloon. Ok, we don't use this currently, but we might in
the future.

If that restricts something you like to do, you can of course remove it.

> 2) What should BALLOON_MULTIPLE_SHOW do? It seems to be unused, and I
> did not understand intended functionality from neither code not
> comment.

If you have a balloon showing for a big window (e.g., the whole client
area in a text editor), then the balloon would show up only once. With
BALLOON_MULTIPLE_SHOW, the balloon will show up again if you move the
mouse, even if you stay within the same window.

> 3) Is BALLOON_TRACK_MOUSE behavior used anywhere? It seems not, but at
> least it is obvious what that one should do.
> I tested it, and found that BALLOON_TRACK_MOUSE has very minor visual
> glitch when combined with BALLOON_SHADOW style. Not sure it is worth
> fixing.

That's why I haven't used it much in TSVN. I like the shadow :)
But the balloons could be used as a normal tooltip replacement (without
shadows), and then the tracking feature is sometimes quite handy.

> 4) What is the intended relationship between BALLOON_INFO::nBehaviour
> and CBalloon::m_nBehaviour? Currently it is rather convoluted, as e.g.
> BALLOON_DIALOG works when set in latter, but not in former, etc.

I think that's a bug :(

> 5) Out of six defined overloads for ShowBalloon function, only one is
> actually used, and each caller passes GetCtrlCentre as a second
> argument.
> I understand that interface was intended to be general, but perhaps it
> is a little overkill? Should I simplify it and reduce the number of
> overloads?

Yes, please do.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org