Review Request 111171: Deprecate (in)active(Title/Text)Color in favor of KColorScheme
Kevin Ottens
ervin at kde.org
Tue Jul 2 08:17:13 UTC 2013
> On July 1, 2013, 2:39 p.m., Kevin Ottens wrote:
> > kdeui/util/kglobalsettings.h, line 265
> > <http://git.reviewboard.kde.org/r/111171/diff/2/?file=166657#file166657line265>
> >
> > I think it's fine. lxr.kde.org seem to indicate that those colors got abused at some places anyway, and what's proposed here seems like a proper replacement to me (except if you're kwin of course, but I think the people behind it know that by now).
>
> Thomas Lübking wrote:
> "@deprecated since 5.0, This used to export the color of the titlebar. If you wanted to query the color of the window background, use KColorScheme(QPalette::Active, KColorScheme::Window).foreground(KColorScheme::ActiveText).color(), otherwise please read the setting from <where it's moved> directly"?
<where it's moved> would be kwin
Yes, sounds good to me.
> On July 1, 2013, 2:39 p.m., Kevin Ottens wrote:
> > khtml/misc/helper.cpp, line 160
> > <http://git.reviewboard.kde.org/r/111171/diff/2/?file=166658#file166658line160>
> >
> > Why unrelated?
> >
> > It's removing calls to the newly deprecated API from KHTML (one of the abusers from my previous comment). Could be made in a separate commit indeed, but for review purpose it seems relevant to the topic for me.
>
> Thomas Lübking wrote:
> The function so far handed the color of the titlebar, now it hands the color of the window.
>
> a) this has been wrong so far (as you suggest):
> then this fixes an unrelated present bug - i personally don't mid too much, but it's still an unrelated bug ;P
> b) this has been correct so far:
> then this introduces a behavioral change, implying titlebar color == window color, which is either intentionally unrelated (only relation being the opportunity) or unintentionally, ie. the change in reaction to the deprecation is wrong and accidentally alters behavior.
>
> To recap my worries and position on this topic:
> If there shall be a global alignment of "titlebar color == window background color" (with the freedom of single decorations to deviate, of course) I want that decision explicitly *taken* and not somehow implied - to avoid any sequel to that oxygen/ozone kindergarten session.
I would be (a) in this case. So yes, would have been better as a separate commit, but that's no big deal IMO.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111171/#review35358
-----------------------------------------------------------
On June 28, 2013, 5:10 p.m., Àlex Fiestas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111171/
> -----------------------------------------------------------
>
> (Updated June 28, 2013, 5:10 p.m.)
>
>
> Review request for KDE Frameworks and kdelibs.
>
>
> Description
> -------
>
> Deprecate: inactiveTitleColor, inactiveTextColor, activeTitleColor, activeTextColor in favor of KColorScheme and replace the implementation of those methods with it.
>
>
> Diffs
> -----
>
> kdeui/util/kglobalsettings.h 4b77ed5
> khtml/misc/helper.cpp dccb9bf
>
> Diff: http://git.reviewboard.kde.org/r/111171/diff/
>
>
> Testing
> -------
>
> I have compared the colors returned by the methods before and after this patch, they are close enough.
>
> Additionally used some apps like filelight with the change, and it seems to work for them as well.
>
>
> Thanks,
>
> Àlex Fiestas
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130702/f3db0dcf/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list