D16767: Add option to toggle all hidden panel item to view menu

[Elvis Angelaccio]

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

INLINE COMMENTS

> dolphinmainwindow.cpp:1338
>  
> +    //connect the action to show Hidden places.
> +    auto actionShowAllPlaces = new QAction(QIcon::fromTheme(QStringLiteral("hint")), i18nc("@item:inmenu", "Show Hidden Places"));

I don't think this comment adds any value, code is already self-explanatory. I'd just remove it.

> dolphinmainwindow.cpp:1339
> +    //connect the action to show Hidden places.
> +    auto actionShowAllPlaces = new QAction(QIcon::fromTheme(QStringLiteral("hint")), i18nc("@item:inmenu", "Show Hidden Places"));
> +    actionShowAllPlaces->setCheckable(true);

Please add `this` as parent, otherwise it will leak (see D16817 <https://phabricator.kde.org/D16817>).

> dolphinmainwindow.cpp:1345
> +    });
> +    //connect the panel event to the action
> +    connect(m_placesPanel, &PlacesPanel::showHiddenEntriesChanged, this, [actionShowAllPlaces, this] (bool checked){

Same, please remove it

> dolphinmainwindow.cpp:1346
> +    //connect the panel event to the action
> +    connect(m_placesPanel, &PlacesPanel::showHiddenEntriesChanged, this, [actionShowAllPlaces, this] (bool checked){
> +        actionShowAllPlaces->setChecked(checked);

We don't need to capture `this` here.

> placespanel.h:60
>      void readSettings() override;
> +    void slotShowHiddenEntries(bool shown);
>  

The `slotXXX` prefix is usually used for private slots. We can just call it `showHiddenEntries()`.

REPOSITORY
  R318 Dolphin

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

To: rizzitello, #dolphin, #vdg, elvisangelaccio
Cc: cfeck, elvisangelaccio, veqz, davidc, ngraham, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20181111/f5c34519/attachment.htm>