----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123682/#review80124 -----------------------------------------------------------
Regressions just going by the screenshot: * Doesn't use form layouting (e.g. the font box labels aren't right-aligned). * The font box height doesn't match the button heights anymore. * Missing colons in the text labels. * Wonky margin between combo box label and combo box. * No keyboard accelerators. General conceptual problems with this port: * Poor QStyle support, e.g. drag on empty space in Oxygen/Breeze doesn't work with this toolkit. * IIRC QStyle background support (like Oxygen's radial gradient) doesn't work either, don't remember though. Additional problems after trying it out: * Poor load performance - loading the KCM from the System Settings main page is significantly slower than the old KCM and involves some flicker. * The anti-aliasing config dialog has a white/unstyled background and improper dialog margins. There's also a ton of crap margins/positioning around widget in its form. * The "Hinting style" combo box popup has wonky margins between the radio buttons and the text labels, and a grey background when it's supposed to be white. What's the motivation behind this port? What user problem does it solve? How does it make this product better? Remember to not just throw code over the fence; you're supposed to argue for why a change is a good idea on review board. Your review request doesn't adequately answer any of these questions. Note that "but other KCMs use Qt Quick and have similar problems" is no argument for making this one worse (but it's an argument for fixing the others by whatever means). - Eike Hein On May 8, 2015, 12:39 p.m., Antonis Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123682/ > ----------------------------------------------------------- > > (Updated May 8, 2015, 12:39 p.m.) > > > Review request for Plasma. > > > Repository: plasma-desktop > > > Description > ------- > > This patch ports the kcm fonts to QML. > > > Diffs > ----- > > kcms/fonts/CMakeLists.txt d73636e > kcms/fonts/fonts.cpp 74da799 > kcms/fonts/fonts.desktop 1cdad40 > kcms/fonts/fonts.h d98bbe2 > kcms/fonts/kcm_fonts.desktop PRE-CREATION > kcms/fonts/package/contents/ui/main.qml PRE-CREATION > kcms/fonts/package/metadata.desktop PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/123682/diff/ > > > Testing > ------- > > Everything works execpt from the ComboBox and the FontDialog ("Configure > Font"). > > * FontDialog > > If you open the kcm inside from the system settings, everything is ok. > > If you use kcmshell5 fonts, the FontDialog is opening behing the kcm window. > In order to solve this issue we must use the setTransientParent, but how can > i do that in the FontDialog? > > * ComboBox > > If you open the kcm with the "kcmshell5 fonts", the dropdown menu renders > fine. > > But if you open it inside from the system settings, the dropdown menu, renders > in the left of the ComboBox. > > > Also these two signals (main.qml line 295) > > onDpiChanged > onAliasingChanged > > are being emitted but the kcm.needsSave doesn't work... > > > File Attachments > ---------------- > > fonts qml > > https://git.reviewboard.kde.org/media/uploaded/files/2015/05/08/833710e7-9569-4cd3-a02f-3ebe95a1c914__kcm_fonts_qml.png > > > Thanks, > > Antonis Tsiapaliokas > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
