pino requested changes to this revision.
pino added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kfileplaceeditdialog.cpp:66
> + // Get icon except for trash
> + if (url.scheme() != QLatin1String("trash")) {
> + icon = dialog->icon();
This duplicates the logic that is used in the constructor below. A better
option might be to create an helper class method to get whether the icon can be
edited, basically checking for `m_iconButton`.
> kfileplaceeditdialog.cpp:123
>
> whatsThisText = i18n("<qt>This is the icon that will appear in the
> Places panel.<br /><br />"
> "Click on the button to select a different
> icon.</qt>");
This variable assignment can be moved inside the `if` as well, since this
"what's this" text is used only for the icon button & its label.
> kfileplaceeditdialog.cpp:132
> + m_iconButton->setIconType(KIconLoader::NoGroup, KIconLoader::Place);
> +
> + if (icon.isEmpty()) {
Extra empty line.
> kfileplaceeditdialog.cpp:138
> + }
> +
> + m_iconButton->setWhatsThis(whatsThisText);
Extra empty line.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D14360
To: shubham, ngraham, broulik, #dolphin, #frameworks, dfaure, pino
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns
