<table><tr><td style="">ngraham requested changes to this revision.<br />ngraham added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D13777">View Revision</a></tr></table><br /><div><div><p>KWidgetsAddons is a Tier 1 framework, so it can't depend on any other KDE Frameworks, including KConfig, which is where the current theme's full palette is stored. Because of that, we're restricted to hardcoding the colors for the Positive, Warning, and Error styles rather than reading them from the current theme. My thinking was that since we're already forced to do this, it made more sense to just hardcode everything and preserve a consistent visual style until and unless we can somehow make it use the theme's colors.</p>
<p>I'm not necessarily against using the palette's highlight color for the Information widget, but I'd strongly prefer a solution that allows the use of all colors from the active theme, not just one.</p>
<p>We can't use the local widget's own palette, for reasons explained below in an inline comment.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13777#inline-72469">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmessagewidget.cpp:262</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// use the current widget's palette (it could be different from the application palette)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span class="n">QPalette</span> <span class="n">palette</span> <span style="color: #aa2211">=</span> <span class="n">palette</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Using the global application palette was deliberate. Try your change and then suspend a terminal window that has a dark background when you're using Breeze light. Local widgets may set a palette that's incompatible with some of the colors we're forced to hardcode due to <tt style="background: #ebebeb; font-size: 13px;">KWidgetsAddons</tt> being a Tier 1 framework, or may set a palette that's incomplete and results in unreadable text. It's much safer to just use the app's own palette.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13777#inline-72471">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmessagewidget.cpp:272</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">case</span> <span style="color: #a0a000">Information</span><span class="p">:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">bgBaseColor</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">setRgb</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #601200"><span class="bright">61</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">174</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">233</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// Window: ForegroundActiv</span>e</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span style="color: #74777d"><span class="bright">// use the highlight colour for this type, as befor</span>e</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">bgBaseColor</span> <span style="color: #aa2211">=</span> <span class="n">palette</span><span class="p">.</span><span class="n">highlight</span><span class="p">().</span><span class="n">color</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">"as before" not needed. Really, <a href="https://phabricator.kde.org/p/cfeck/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@cfeck</a> is right and the entire comment isn't needed.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13777#inline-72470">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmessagewidget.cpp:282</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">const</span> <span class="n">QPalette</span> <span class="n">palette</span> <span style="color: #aa2211">=</span> <span class="n">QGuiApplication</span><span style="color: #aa2211">::</span><span class="n">palette</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">const</span> <span class="n">QColor</span> <span class="n">windowColor</span> <span style="color: #aa2211">=</span> <span class="n">palette</span><span class="p">.</span><span class="n">window</span><span class="p">().</span><span class="n">color</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R236 KWidgetsAddons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D13777">https://phabricator.kde.org/D13777</a></div></div><br /><div><strong>To: </strong>rjvbb, ngraham, Frameworks<br /><strong>Cc: </strong>cfeck, kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>