When going to adapt some code to follow the deprecation, I now look though at some inconsistency in the API:

• KTitleWidget has a property "pixmap"
• the setPixmap methods all set this pixmap, with overloads for convenience
• setIcon is without a getter and a documented property, the relation to the pixmap property is unclear just by the API names

With this in mind, I fear I consider this change a regression in API quality. I'd propose to revert this, or go the full mile and make "icon" a real property, deprecating the pixmap one. What do you think?

In D23927#537583, @kossebau wrote:

When going to adapt some code to follow the deprecation, I now look though at some inconsistency in the API:

• KTitleWidget has a property "pixmap"
• the setPixmap methods all set this pixmap, with overloads for convenience
• setIcon is without a getter and a documented property, the relation to the pixmap property is unclear just by the API names
  
  With this in mind, I fear I consider this change a regression in API quality. I'd propose to revert this, or go the full mile and make "icon" a real property, deprecating the pixmap one. What do you think?

Moving even more towards a QIcon rather than a QPixmap API is a good thing for sure, I agree on that. I don't think going back is justified here though, especially since the QIcon-based API improves high DPI scalability support. If it's decided to revert this nevertheless, please note that this has a ripple effect on the frameworks already depending on this.

In D23927#537585, @vkrause wrote:

Moving even more towards a QIcon rather than a QPixmap API is a good thing for sure, I agree on that. I don't think going back is justified here though, especially since the QIcon-based API improves high DPI scalability support. If it's decided to revert this nevertheless, please note that this has a ripple effect on the frameworks already depending on this.

The commit & its message claims to "Improve naming of KTitleWidget icon methods", which it does not. It is now actually resulting in wrong expectations.

Using a QIcon as pixmap provider engine surely makes sense. That is why the old API had those overloads. But the method does not set an icon, it sets the pixmap by immediately having the QIcon generating the pixmap and then discarding the QIcon object. Which means, if the title widget gets scaled e..g by setting another level, the API user had no gain by passing a QIcon object. {Edit; okay, scaling is not documented and also not supported, so QIcon does not gain us here something. but support for screen dpi changes would be expected wen "setting" a QIcon],

Please add a respective disclaimer to the API documentation for now, as the current documentation is misleading, as I can tell you by experience :)

Any chance you could pik this up and complete? Thing is, the deprecated API is also still used internally,so anything but deprecated. Actually the new API calls the "deprecated" one.

I just tried to adapt the code to the new deprecation macros (forgot that this change has slipped in since I did D24468 and landed it).
But I failed due to this reason, as #if KWIDGETSADDONS_BUILD_DEPRECATED_SINCE(5, 63) cannot be used for KTitleWidget::setPixmap() implementation due to that.

In D23927#548607, @kossebau wrote:

Any chance you could pik this up and complete? Thing is, the deprecated API is also still used internally,so anything but deprecated. Actually the new API calls the "deprecated" one.

Hmpf, I mixed up signatures and added "#if KWIDGETSADDONS_BUILD_DEPRECATED_SINCE(5, 63)" to the wrong method, all fine there, sorry for that noise.

Still, complain about the API consistency still holds, please consider fixing it :)