Review Request 112375: Correctly determine frame width when called from QtQuickControls

[Hugo Pereira Da Costa]

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



kstyles/oxygen/oxygenstyle.cpp
<http://git.reviewboard.kde.org/r/112375/#comment28774>

    Rather than looking up the property multiple times, alongside the q_object_cast, I would rather add the test just before the last 'if' (when everything else has failed), and possibly caching the property in a QString. 
    
    Something like
    
    else if( !widget && option && option->styleObject ) {
        const QString elementType = option->styleObject->property( elementType );
    
        ... your code (all ifs, duplicated from the widget case)
    
    } else { 
    
        return 1;
    
    }
    
    This way, you don't lookup the property if the widget is defined,
    and You don't lookup the property multiple times. 
    This comes at the price of some code duplication, but it is a small block of code, so I'd deem that acceptable.
    
    Oppinion ?


- Hugo Pereira Da Costa


On Aug. 30, 2013, 3:56 p.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, 3:56 p.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Description
> -------
> 
> Correctly determine frame width when called from QtQuickControls
> 
> When oxygen is used in QtQuickControls the widget parameter is empty
> as such we need to determine the widget type (lineedit etc. ) in a
> different way
> 
> 
> Diffs
> -----
> 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130831/412ea422/attachment.html>