D14360: Remove custom icon selection for trash

Pino Toscano noreply at phabricator.kde.org
Sat Jul 28 05:24:53 BST 2018


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180728/63d821a0/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list