<table><tr><td style="">rkflx 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/D12849">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D12849#263660" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D12849#263660</a>, <a href="https://phabricator.kde.org/p/progwolff/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@progwolff</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>That would be this one:</p>
<blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D12849#262919" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D12849#262919</a>, <a href="https://phabricator.kde.org/p/progwolff/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@progwolff</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p><a href="https://phabricator.kde.org/F5849709" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F5849709: Screenshot_fonts_layout_tabbar.png</a></p></div>
</blockquote>
<p><a href="https://phabricator.kde.org/p/mart/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@mart</a> <a href="https://phabricator.kde.org/p/rkflx/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@rkflx</a> <a href="https://phabricator.kde.org/p/broulik/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@broulik</a> is this okay for you?</p></div>
</blockquote>
<p>I like it, thanks for the refinement ;)</p>
<p>Two more things I noticed which might be relevant for the string freeze:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">The <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Choose a font</span></span></span> window title should use title case (possibly in a separate commit).</li>
<li class="remarkup-list-item"><span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Disabled</span></span></span> vs. <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Disabled from</span></span></span> (which actually means "Enabled", as can be seen by the rest of the options not showing the "disabled" state) might be a bit confusing, and I'm also not sure how well that "word puzzle" sits with translators. Maybe using <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Exclude range:</span></span></span> (i.e. different wording and with a colon) would work better?</li>
</ul>
<p>Did not review the code, but I noticed some warnings in addition to those mentioned above:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">share/kpackage/kcms/kcm_fonts/contents/ui/main.qml:230:25: QML Image: Invalid image provider: image://preview/0_0.png
…
share/kpackage/kcms/kcm_fonts/contents/ui/main.qml:273:17: QML SpinBox: Binding loop detected for property "value"</pre></div>
<hr class="remarkup-hr" />
<p>Not really related to this patch, but it would be great if you could work on or file bugs for some of the following issues (not sure whether Kirigami or Fonts KCM problem):</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">Preview comboboxes: Weird vertical placement of the popup (compared to QWidget comboboxes).</li>
<li class="remarkup-list-item">Preview comboboxes: Clicking outside does not close the popup.</li>
<li class="remarkup-list-item">Radio buttons: Horizontal spacing between radio button and text on the right is too large compared to the QWidget counterpart.</li>
</ul></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>BRANCH</strong><div><div>fonts_kcm_layout (branched from master)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D12849">https://phabricator.kde.org/D12849</a></div></div><br /><div><strong>To: </strong>progwolff, mart, abetts, ngraham<br /><strong>Cc: </strong>broulik, zzag, rkflx, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>