[wx-dev] wxTaskBarIcon::SetIcon enhancement

[David Barnard]

I tried using ChangeIcon(), but I encountered a problem.

SetIcon() currently doesn't just change the icon, it actually adds the
icon to the taskbar for the first time. Having this functionality
available through SetIcon() and ChangeIcon() would be highly
confusing.

Is there any reason why wxTaskBarIcon doesn't use the same
constructor/Create() logic used by other controls?

Here's a new suggestion. Deprecate SetIcon(). Add a constructor and
create method:

wxTaskBarIcon::wxTaskBarIcon();
wxTaskBarIcon::wxTaskBarIcon(const wxIcon& icon,
                         const wxString& tooltip = wxEmptyString);
bool wxTaskBarIcon::Create(const wxIcon& icon,
                         const wxString& tooltip = wxEmptyString);

and use the ChangeX functions you suggested:

bool wxTaskBarIcon::ChangeIcon(const wxIcon& icon);
bool wxTaskBarIcon::ChangeTooltip(const wxString& tooltip);

I think this conforms to the "principle of least astonishment". Also,
it can be written to not break existing code. What do you think?

David Barnard

On Sat, Jul 12, 2008 at 10:06 PM, David Barnard <david at didactylos.net> wrote:
> Just to be clear: you prefer a minor breaking change (removing the
> default value) than a minor change in (undocumented) functionality?
>
> David Barnard
>
> On Sat, Jul 12, 2008 at 4:21 PM, Marcin Wojdyr <wojdyr at gmail.com> wrote:
>> On Sat, Jul 12, 2008 at 3:43 PM, Vadim Zeitlin <vadim at wxwidgets.org> wrote:
>>>
>>> DB> I propose adding two extra functions:
>>> DB> wxTaskBarIcon::SetIcon(const wxIcon &icon)
>>>
>>>  This would conflict with the existing one though, unless you remove the
>>> default value for the tooltip parameter. But even if you do, the existing
>>> code using SetIcon(icon) would now work differently as it wouldn't reset
>>> the tooltip any more. So I think we need a separate function, e.g.
>>> ChangeIcon().
>>>
>>> DB> wxTaskBarIcon::SetTitle(const wxString &tooltip)
>>>
>>>  This one would be ok but for consistency with the other one it should be
>>> called ChangeTooltip() too.
>>>
>>> DB> And leaving the existing overload, but removing the default parameter.
>>> DB> This won't break existing code,
>>>
>>>  See above, it will change its behaviour.
>>
>> David has a good point that changing tooltip to empty string by
>> SetIcon(icon) may not be expected by users.
>> Default value for tooltip in SetIcon() is not documented, so IMHO it
>> should be removed to avoid confusion.
>> We would have two functions SetIcon(icon, tooltip) and ChangeIcon(icon).
>>
>> Marcin
>>
>> --
>> Marcin Wojdyr | http://www.unipress.waw.pl/~wojdyr/
>> _______________________________________________
>> wx-dev mailing list
>> wx-dev at lists.wxwidgets.org
>> http://lists.wxwidgets.org/mailman/listinfo/wx-dev
>>
>