From kde-panel-devel Sat Jan 31 23:58:29 2015 From: "David Edmundson" Date: Sat, 31 Jan 2015 23:58:29 +0000 To: kde-panel-devel Subject: Re: Review Request 122332: KQuickControls IconDialog Message-Id: <20150131235829.13680.55908 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=142274872221198 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1395774752903302783==" --===============1395774752903302783== Content-Type: multipart/alternative; boundary="===============7889490327269793990==" --===============7889490327269793990== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit On Jan. 31, 2015, 3:41 a.m., Kai Uwe Broulik wrote: > > I told you previously but I think you missed it, we have PlasmaPlatformComponents.IconDialog, so lets not reinvent wheel. it is already used by activity manager We cannot at the same time tell people Plasma components and app components should be visually different; and at the same time tell developers they should be using some stuff from plasma in their apps. They're not compatiable plans. Also this is a /lot lot/ better than the implementation in PlatformComponents. It's similar to the approach QtQuick.Dialogs; you return a QObject with properties. For now that wraps a QWidget dialog in the desktop mode for now; but it could just as easily return something a QWindow QML item with QtQuick Controls inside in future without breaking API. (much) It might be re-inventing a wheel, but it's adding a rubber tyre on the outside whilst it does it. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122332/#review75062 ----------------------------------------------------------- On Jan. 30, 2015, 10:07 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122332/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2015, 10:07 p.m.) > > > Review request for Plasma and Daniel Vrátil. > > > Repository: kdeclarative > > > Description > ------- > > This patch adds KQuickControls wrapper around KIconDialog similar to how the ColorDialog and other QtQuick Dialogs work. This can be used, for instance, in Kickoff's config UI to provide a picker for a custom item. > > It is an initial draft and lacks for example window modality as I couldn't figure out how QtQuick Dialogs do that (some PlatformDialogHelper magic inside) and I'm also not sure about the lifecycle/ownership of the dialog, I've seen a lot of fixes for issues in that area on Review Board. > > > Diffs > ----- > > src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 65e28ff > src/qmlcontrols/kquickcontrolsaddons/icondialog.h PRE-CREATION > src/qmlcontrols/kquickcontrolsaddons/icondialog.cpp PRE-CREATION > src/qmlcontrols/kquickcontrolsaddons/kquickcontrolsaddonsplugin.cpp 289f1ed > > Diff: https://git.reviewboard.kde.org/r/122332/diff/ > > > Testing > ------- > > For testing I added a button to Kickoff that allows to open the dialog and the button icon source is bound to the dialog's iconName property. Didn't test the icon source/user/custom path stuff. > > > Thanks, > > Kai Uwe Broulik > > --===============7889490327269793990== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122332/

On January 31st, 2015, 3:41 a.m. UTC, Bhushan Shah wrote:

I told you previously but I think you missed it, we have PlasmaPlatformComponents.IconDialog, so lets not reinvent wheel. it is already used by activity manager

We cannot at the same time tell people Plasma components and app components should be visually different; and at the same time tell developers they should be using some stuff from plasma in their apps. They're not compatiable plans.
Also this is a /lot lot/ better than the implementation in PlatformComponents. It's similar to the approach QtQuick.Dialogs; you return a QObject with properties. For now that wraps a QWidget dialog in the desktop mode for now; but it could just as easily return something a QWindow QML item with QtQuick Controls inside in future without breaking API. (much)
It might be re-inventing a wheel, but it's adding a rubber tyre on the outside whilst it does it.

- David

On January 30th, 2015, 10:07 p.m. UTC, Kai Uwe Broulik wrote:

Review request for Plasma and Daniel Vrátil.

By Kai Uwe Broulik.

Updated Jan. 30, 2015, 10:07 p.m.

Repository: kdeclarative

Description

This patch adds KQuickControls wrapper around KIconDialog similar to how the ColorDialog and other QtQuick Dialogs work. This can be used, for instance, in Kickoff's config UI to provide a picker for a custom item.

It is an initial draft and lacks for example window modality as I couldn't figure out how QtQuick Dialogs do that (some PlatformDialogHelper magic inside) and I'm also not sure about the lifecycle/ownership of the dialog, I've seen a lot of fixes for issues in that area on Review Board.

Testing

For testing I added a button to Kickoff that allows to open the dialog and the button icon source is bound to the dialog's iconName property. Didn't test the icon source/user/custom path stuff.

Diffs

src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt (65e28ff)
src/qmlcontrols/kquickcontrolsaddons/icondialog.h (PRE-CREATION)
src/qmlcontrols/kquickcontrolsaddons/icondialog.cpp (PRE-CREATION)
src/qmlcontrols/kquickcontrolsaddons/kquickcontrolsaddonsplugin.cpp (289f1ed)

--===============7889490327269793990==-- --===============1395774752903302783== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============1395774752903302783==--