D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
This revision was automatically updated to reflect the committed changes. kossebau marked 2 inline comments as done. Closed by commit R263:415402081d92: Add KAboutPluginDialog, to be used with KPluginMetaData (authored by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D25058?vs=70052=70258#toc REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25058?vs=70052=70258 REVISION DETAIL https://phabricator.kde.org/D25058 AFFECTED FILES src/CMakeLists.txt src/kaboutapplicationdialog.cpp src/kaboutapplicationdialog.h src/kaboutplugindialog.cpp src/kaboutplugindialog.h src/kabstractaboutdialog_p.cpp src/kabstractaboutdialog_p.h src/klicensedialog_p.cpp src/klicensedialog_p.h To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> kaboutapplicationdialog.cpp:72
> q->setWindowTitle(i18n("About %1", aboutData.displayName()));
> q->setModal(false);
>
(this line does nothing, dialogs aren't modal by default)
> kaboutplugindialog.cpp:78
> +q->setWindowTitle(i18n("About %1", pluginMetaData.name()));
> +q->setModal(false);
> +
(this line does nothing, dialogs aren't modal by default)
REPOSITORY
R263 KXmlGui
BRANCH
addaboutplugindialog
REVISION DETAIL
https://phabricator.kde.org/D25058
To: kossebau, #frameworks, apol, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
kossebau updated this revision to Diff 70052. kossebau marked 2 inline comments as done. kossebau added a comment. Update to first review by @dfaure REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25058?vs=69002=70052 BRANCH addaboutplugindialog REVISION DETAIL https://phabricator.kde.org/D25058 AFFECTED FILES src/CMakeLists.txt src/kaboutapplicationdialog.cpp src/kaboutapplicationdialog.h src/kaboutplugindialog.cpp src/kaboutplugindialog.h src/kabstractaboutdialog_p.cpp src/kabstractaboutdialog_p.h src/klicensedialog_p.cpp src/klicensedialog_p.h To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
kossebau marked 3 inline comments as done. kossebau added inline comments. INLINE COMMENTS > dfaure wrote in kaboutplugindialog.cpp:119 > (I'm surprised by the explicit '&' in all those labels, doesn't > KAcceleratorManager take care of this automatically?) Seems it does. So removing then while moving the code. > dfaure wrote in klicensedialog_p.cpp:72 > Well, the code calls `resize` and then `adjustSize`, which calls `resize` > itself. So at least one issue is that one of those two calls is redundant. > Try removing `adjustSize`? Thanks for investigating. Seems indeed removing `adjustSize` brings (back) the intended size :) REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D25058 To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
dfaure added a comment.
Nice work.
INLINE COMMENTS
> kaboutplugindialog.cpp:119
> +QWidget *translatorWidget =
> createTranslatorsWidget(pluginMetaData.translators(), QString(), q);
> +tabWidget->addTab(translatorWidget, i18n("T"));
> +}
(I'm surprised by the explicit '&' in all those labels, doesn't
KAcceleratorManager take care of this automatically?)
> kabstractaboutdialog_p.h:40
> +public:
> +QWidget * createTitleWidget(const QPixmap ,
> +const QString ,
Coding style: there should be no space after the *, right?
> klicensedialog_p.cpp:72
> +
> +// TODO: this does not work anymore
> +resize(sizeHint().expandedTo(QSize(qRound(idealWidth), idealHeight)));
Well, the code calls `resize` and then `adjustSize`, which calls `resize`
itself. So at least one issue is that one of those two calls is redundant. Try
removing `adjustSize`?
REPOSITORY
R263 KXmlGui
REVISION DETAIL
https://phabricator.kde.org/D25058
To: kossebau, #frameworks, apol, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
kossebau added a dependent revision: D25063: Deprecate KAboutData::fromPluginMetaData, now there is KAboutPluginDialog. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D25058 To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
kossebau added a dependent revision: D25059: KPluginSelector: use new KAboutPluginDialog. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D25058 To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25058: Add KAboutPluginDialog, to be used with KPluginMetaData
kossebau created this revision. kossebau added reviewers: Frameworks, apol, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY When showing an About dialog for a plugin, so far a KAboutApplicationDialog had to be used, by generating an KAboutData object from the KPluginMetaData object. Using KAboutApplicationDialog has those disadvantages: - the icon shown in the title widget is the one of the application, not the plugin - the "Libraries" tab does not make much sense with a plugin, as it is the same as the shell application - the name KAboutApplicationDialog is a mismatch when used with a plugin TEST PLAN About application dialog works as before in applications, the new about plugin dialog works with KDevelop's Loaded Plugins dialog as well as KPluginSelector. REPOSITORY R263 KXmlGui BRANCH addaboutplugindialog REVISION DETAIL https://phabricator.kde.org/D25058 AFFECTED FILES src/CMakeLists.txt src/kaboutapplicationdialog.cpp src/kaboutapplicationdialog.h src/kaboutplugindialog.cpp src/kaboutplugindialog.h src/kabstractaboutdialog_p.cpp src/kabstractaboutdialog_p.h src/klicensedialog_p.cpp src/klicensedialog_p.h To: kossebau, #frameworks, apol, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
