rkflx added a comment.
In D12849#263660 <https://phabricator.kde.org/D12849#263660>, @progwolff wrote: > That would be this one: > > In D12849#262919 <https://phabricator.kde.org/D12849#262919>, @progwolff wrote: > > > F5849709: Screenshot_fonts_layout_tabbar.png <https://phabricator.kde.org/F5849709> > > > @mart @rkflx @broulik is this okay for you? I like it, thanks for the refinement ;) Two more things I noticed which might be relevant for the string freeze: - The Choose a font window title should use title case (possibly in a separate commit). - Disabled vs. Disabled from (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 Exclude range: (i.e. different wording and with a colon) would work better? Did not review the code, but I noticed some warnings in addition to those mentioned above: 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" --- 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): - Preview comboboxes: Weird vertical placement of the popup (compared to QWidget comboboxes). - Preview comboboxes: Clicking outside does not close the popup. - Radio buttons: Horizontal spacing between radio button and text on the right is too large compared to the QWidget counterpart. REPOSITORY R119 Plasma Desktop BRANCH fonts_kcm_layout (branched from master) REVISION DETAIL https://phabricator.kde.org/D12849 To: progwolff, mart, abetts, ngraham Cc: broulik, zzag, rkflx, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
