-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112375/#review38928
-----------------------------------------------------------


I respectfully disagree with the change.

If I understand the descripion right, the only thing that matters for QtQuick 
is to change the default returned value (if all else fail) from 1 to 
FRAME_FRAMEWIDTH. Correct ?

The other enumerations, (LineEdit_FrameWidth, ComboBox_FrameWidth, etc.) must 
stay: even if they have the same values at the moment, this might change in the 
future (in the near future in fact, since nuno and I want to revisit the 
metrics, make them more dpi independent, etc.).

That the cast wont work for QtQuick is not a good reason to remove it, with all 
(non quick) applications around, and for which such casts work. 

Now as for the change needed for QtQuick, this will (must) break (that is: 
change) all applications that render a frame and don't fall in the cathegories 
above (think custom widgets). I too have no example of this (but I'm sure there 
are), and since these are "custom" things, it is normal that they don't show up 
in oxygen-demo.

Please update the change to the "minimal" (namely -> change the default value 
returned for PM_DefaultFrameWidth), and then, well, we'll need to test ...


- Hugo Pereira Da Costa


On Aug. 30, 2013, 11:11 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112375/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 11:11 a.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Description
> -------
> 
> Use a single frame width for all PM_DefaultFrameWidth
> 
> The current code sets a width of 3 for all line edits, combo boxes
> and frames, otherwise it returns a width of 1.
> 
> The QtQuickControls engine cannot qobject_cast() the widget so always
> return a frame width of 1 for qtquickcontrol line edits and combo boxes.
> 
> This simplifies the code and solves that issue.
> 
> I can think of no other way to resolve this without editing Qt, and even then 
> it would be difficult to extend the PixelMetric enum without breaking 
> compatibility.
> 
> Note this is potentially a visual change in oxygen, however I have yet to see 
> anything actually different.
> 
> 
> Diffs
> -----
> 
>   kstyles/oxygen/oxygenmetrics.h 0643ae5b20d0c9efa328a87e08707cebaabf9f5e 
>   kstyles/oxygen/oxygenstyle.cpp 86b5cdf3054f5d362d90f0f76c30bfb4f2646911 
> 
> Diff: http://git.reviewboard.kde.org/r/112375/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> QML After
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/30/spell1.png
> QML_Before
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/30/spell1_1.png
> Normal oxygen demo
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/30/oxygen1.png
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to