Review Request 123682: Plasma-Desktop: Port kcm fonts to QML.

Mon May 11 12:57:51 UTC 2015


> On May 9, 2015, 1:18 p.m., Eike Hein wrote:
> > 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 wrote:
>     Sorry, the above was destroyed by markdown parsing:
>     
>     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).
> 
> Marco Martin wrote:
>     so, we talked a bit about it in hangout, the solution will be:
>     * do a new branch of master (called like QmlKCMs)
>     * merge this and the cursortheme one in such branch
>     * do any eventual new one in the branch
>     * master stays as it is for the next future
>     * (even ask distributions to package such branch as an experimental, non default alternative to make testing easy)
>     
>     any eventual qml/qtquickcontrols but that comes out from doing that is attempted to be fixed upstream
>     the one big branch will be merged when good enough.

^ Sounds good


- Eike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123682/#review80124
-----------------------------------------------------------


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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150511/00bb5f04/attachment.html>