<table><tr><td style="">rjvbb added a comment.
</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/D5111" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">please re-add the screenshot from Review Board. (sorry I was not aware of this review request cause I was not in the list of reviewers, even though official maintainer of oxygen ...)</li>
</ul></blockquote>

<p>Sorry about that, I thought you'd be a member of the Plasma group. But I had a hunch so added a few reviewers manually this time.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">right to left layout action <- no. There is already one at the bottom of the window.</li>
</ul></blockquote>

<p>The goal was not to duplicate functionality, but to add an additional item to the Layout menu that was not mutually exclusive with the others, as different styles can render such items differently. It also gives the possibility to add a separator.<br />
IIRC the patch *would* become somewhat simpler if I just leave the menu item but make it a noop. Would you be OK with that?</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">this review should really be several, one per feature: one for the checkboxes/radiobuttons in the mdi window, one for the colorschemechooser. Can you split ?</li>
</ul></blockquote>

<p>Yay, extra work... Fortunately not too much, will do later today.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">finally, there is need for more detail review (once above is done). For instance, in ColorSchemeChooser you use SUPPORT_THEME_SAVING, but this one is set/defined nowhere.</li>
</ul></blockquote>

<p>TBH, I was more or less hoping for feedback like this. I also don't see the need to save this setting but didn't want to strip it out immediately, also out of some form of respect for Alexander's original work.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R113 Oxygen Theme</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5111" rel="noreferrer">https://phabricator.kde.org/D5111</a></div></div><br /><div><strong>To: </strong>rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta<br /><strong>Cc: </strong>kde-mac, Frameworks<br /></div>