> On Aug. 30, 2013, 12:43 p.m., Hugo Pereira Da Costa wrote:
> > 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 ...
> >
> 
> David Edmundson wrote:
>     I expected this to be a review which started a discussion :)
>     It's one that sets precedent for a few other things.
>     
>     The thing that matters is for QtQuick to get the same width as QLineEdits 
> get. This happens to be FRAME_FRAMEWIDTH now.
>     
>     The reason I removed the LineEdit_FrameWidth is that if someone later 
> does decide to change the LineEdit_FrameWidth the QtQuickControls will become 
> broken again not matching the desktop counterpart. I'm deliberately taking 
> away that option.
>     
>     The only way we can support QtQuickControls is to support the lowest 
> common denominator of QStyle that they both follow even if it makes the 
> desktop worse (or at least different). 
>     This basically means supporting only values in QStyle, and not hacking in 
> our own extra granularity determined by casting widgets.
>     
>     I appreciate this is a very sucky situation to be in, but if we don't we 
> will we see a very clear discrepancy in the UI depending on which technology 
> happens to be used. I don't think we want that.
>     
>     By only other options are:
>     --
>     I could try to add PM_LineEdit and PM_ComboBox into Qt5.2? It would solve 
> this particular issue.
>     The base implementation of QStyle can then just call PM_DefaultFrameWidth 
> for backwards compatibility. I'm not sure it would get in as it relies on all 
> styles calling the base implementation for enum values they don't support.
>     --
>     We make a QML Oxygen theme from 'scratch' a bit like how we have an 
> Oxygen GTK theme.. different tech that happens to look the same.
> 
> David Edmundson wrote:
>     s/By/My

Why do QtQuickControls need pixelMetric() for line edits? To find out, how much 
margin there is around a styled line edit, it should simply use 
sizeFromContents() and subElementRect().


- Christoph


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


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