D7941: use Kirigami Theme for colors

David Edmundson noreply at phabricator.kde.org
Mon Sep 25 08:52:23 UTC 2017


davidedmundson added a comment.


  This is set to be a framework.
  We can't have a framework that imports something from workspace.

INLINE COMMENTS

> Label.qml:32
>  
> -    height: Math.round(Math.max(paintedHeight, StylePrivate.TextSingleton.height * 1.6))
> +    height: Math.round(Math.max(paintedHeight, Kirigami.Units * 1.6))
>      verticalAlignment: lineCount > 1 ? Text.AlignTop : Text.AlignVCenter

This change is unrelated. 
Also I said in a review about how we shouldn't be doing this.

> kquickstyleitem.cpp:167
> +        Q_ASSERT(m_theme);
> +        m_theme->setInherit(true);
> +

I don't understand what this is doing? It looks wrong?

> kquickstyleitem.cpp:709
>  
> -    m_styleoption->palette = QApplication::palette(classNameForItem());
> +    //m_styleoption->palette = QApplication::palette(classNameForItem());
> +    //m_theme->setColorSet(Kirigami::PlatformTheme::Complementary);

This area needs some tidying.

Also we need this line if the palette isn't explicitly set to something else.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

REVISION DETAIL
  https://phabricator.kde.org/D7941

To: mart, #plasma, #kirigami, hein
Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170925/fe7ddc35/attachment-0001.html>


More information about the Plasma-devel mailing list