----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122332/#review77930 -----------------------------------------------------------
In general +100. Two minor comments. src/qmlcontrols/kquickcontrolsaddons/icondialog.h (line 61) <https://git.reviewboard.kde.org/r/122332/#comment53433> I don't see this being set to false when the window is closed. src/qmlcontrols/kquickcontrolsaddons/icondialog.cpp (line 156) <https://git.reviewboard.kde.org/r/122332/#comment53432> kicondialog has a weird showDialog method, that does some extra focus things. You should use that (and/or we should fix kicondialog to not be weird) - David Edmundson On March 14, 2015, 9:41 a.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122332/ > ----------------------------------------------------------- > > (Updated March 14, 2015, 9:41 a.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 d5713a0 > src/qmlcontrols/kquickcontrolsaddons/icondialog.h PRE-CREATION > src/qmlcontrols/kquickcontrolsaddons/icondialog.cpp PRE-CREATION > src/qmlcontrols/kquickcontrolsaddons/kquickcontrolsaddonsplugin.cpp cee2360 > > 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
