D19832: Fix a todo: InformationPanelContent::configureSettings code is moved to InformationPanel::contextMenuEvent

Elvis Angelaccio noreply at phabricator.kde.org
Sun Mar 17 17:30:20 GMT 2019


elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> informationpanel.cpp:171-216
> +    QMenu popup(this);
> +
> +    QAction* previewAction = popup.addAction(i18nc("@action:inmenu", "Preview"));
> +    previewAction->setIcon(QIcon::fromTheme(QStringLiteral("view-preview")));
> +    previewAction->setCheckable(true);
> +    previewAction->setChecked(InformationPanelSettings::previewsShown());
> +

I'd put this code in a private function.

> informationpanelcontent.cpp:51
>  #include "dolphin_informationpanelsettings.h"
>  #include "filemetadataconfigurationdialog.h"
>  #include "phononwidget.h"

This can also be dropped now.

> informationpanelcontent.h:75
>  
> -    /**
> -     * Opens a menu which allows to configure which meta information
> -     * should be shown.
> -     *
> -     * TODO: Move this code to the class InformationPanel
> -     */
> -    void configureSettings(const QList<QAction*>& customContextMenuActions, const QPointF& pos);
> +    void setPreviewVisible(bool);
> +

Please add a variable as argument

> informationpanelcontent.h:77
> +
> +    const KFileItemList items();
>  

We don't usually add `const` when a function returns a `QList`.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D19832

To: meven, elvisangelaccio, #dolphin
Cc: kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190317/b71d45ed/attachment.htm>


More information about the kfm-devel mailing list