[Differential] [Commented On] D4508: Plasma controls based on QtQuickControls2

[David Edmundson]

davidedmundson added a comment.


  Generally good to go. Just one major gripe about Label, and making sure our installed items and qmldir are in sync.

INLINE COMMENTS

> CMakeLists.txt:11
> +
> +#install the componenbts as a QQC2 Style, as style for applications (mainly for Plasma Mobile)
> +install(DIRECTORY plasmacomponents3/ DESTINATION ${KDE_INSTALL_QMLDIR}/QtQuick/Controls.2/Plasma PATTERN qmldir EXCLUDE)

typo

> CMakeLists.txt:42
> +
> +    #maybe we really don't want the switch control in Plasma Desktop
> +    plasmacomponents3/SwitchDelegate.qml

It's easier to add something later than to remove it.

I'd go with not installing for now, then revisit it. Maybe same for Menus.

> davidedmundson wrote in Label.qml:27
> Now we have an API break, lets fix this.
> 
> As soon as you put this in a layout it won't do anything, so it's quite inconsistent.
> If anything it should be an implicitHeight, or just set the lineHeight?
> 
> But I'd rather we didn't have it at all, we have to work round it in lots of places.
> 
> If we want a padded Label, we can make a new subclass in the import with all the other headings.

I still want this changed.

> qmldir:11
> +DialogButtonBox 3.0 DialogButtonBox.qml
> +Dialog 3.0 Dialog.qml
> +Dial 3.0 Dial.qml

Dialog and DialogButtonBox are listed here but aren't installed. I think that will cause an error.

And do we need CheckIndicator here?

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: davidedmundson, broulik, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170223/98c29003/attachment.html>