Added Hide Read Feed option
ClosedPublic

Authored by sagara on Oct 1 2017, 6:06 PM.

Details

Summary

This patch adds a new option, to hide feeds with zero unread count from the feed list. You can access the setting via either the View menubar item or via the General configuration panel, it defaults to being turned off. This is useful if you have many feeds, as it makes the feed list less crowded by hiding those with no new items.

subscriptionlistmodel:

  • filter models are present in articlelistview, but controller only includes listmodel.h, so I put FilterUnreadProxyModel here
  • added FilterUnreadProxyModel, a QSortFilerProxyModel subclass to filter out feeds with zero unread count
  • uint nodeIdForIndex(const QModelIndex&) is now a file-specific method
  • changed FolderExpansionHandler::m_model to be of type QAbstractItemModel* (since it no longer needs SubscriptionListModel::nodeIdForIndex)

selectioncontroller:

  • hooked filter's selectionmodel's selectionChanged signal in SelectionController::setFeedSelector to the FilterUnreadProxyModel instance
  • m_subscriptionModel is now a FilterUnreadProxyModel*
  • added SelectionController::settingsChanged slot
  • changed SelectionController::setFeedList to set the proxy's source
  • model

actionmanagerimpl:

  • added slotSettingsChanged slot for receiving MainWidget::signalSettingsChange signal and updating action state
  • added a KAction to ActionManagerImpl:initSubscriptionListView, connected to SubscriptionListView::slotSetHideReadFeeds

subscriptionlistview:

  • added a public slot to connect the added action to (slotSetHideReadFeeds)

mainwidget:

  • hooked m_part's signalSettingsChanged() to m_selectionController::settingsChanged, m_actionManager::slotSettingsChanged

interfaces/akregator.kcfg:

  • added View/HideReadFeeds entry

akregator_part.rc:

  • add a separator and an action to <Menu name="view"> (name=feed_hide_read)

settings_general.ui:

  • add kcfg_HideReadFeeds field under General->Global

Diff Detail

Repository
R201 Akregator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sagara created this revision.Oct 1 2017, 6:06 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 1 2017, 6:06 PM
Restricted Application added a subscriber: KDE PIM. · View Herald Transcript
dvratil requested changes to this revision.Oct 1 2017, 8:06 PM
dvratil added a subscriber: dvratil.

Thanks for the patch, I like this feature. Did not test the patch but generally, it looks fairly good. There are mostly some coding-style nitpicks.

interfaces/akregator.kcfg
9

Missing "articles" at the end?

src/actions/actionmanagerimpl.cpp
127

{ on a new line

130

qCCritical(AKREGATOR_LOG)

src/subscription/subscriptionlistmodel.cpp
96

Use brackets ({ and } even with one-line statements

104

Is this recursive? In other words, does this return true even if idx is a folder with an unread feed? Otherwise you'll need to traverse the hierarchy here in order not to hide those folders.

src/subscription/subscriptionlistmodel.h
44

{ on a new line

48

nullptr instead of 0

50

{ and } on separate lines, ideally un-inline

51

un-inline

54
  • missing override
  • un-inline this function
src/subscription/subscriptionlistview.cpp
399

qCCritical(AKREGATOR_LOG)

This revision now requires changes to proceed.Oct 1 2017, 8:06 PM
sagara updated this revision to Diff 20214.Oct 1 2017, 9:17 PM
sagara marked 11 inline comments as done.
  • Code style fixes
src/subscription/subscriptionlistmodel.cpp
104

Yes this works with nested folders just fine.

sagara added a comment.EditedOct 1 2017, 9:20 PM

I updated the style issues. I originally wrote this patch back in 2011 back when the code didn't have a consistent style, so I missed some stuff when I updated it for submission, sorry about that.

There are still a few one-line if statements with missing brackets in Akregator::FilterUnreadProxyModel, please fix those as well.

I updated the style fixes. I originally wrote this patch back in 2011 back when the code didn't have a consistent style, so I missed some stuff when I updated it for submission, sorry about that.

No worries, that's why we have code review :-)

src/subscription/subscriptionlistmodel.cpp
126

!desel.isEmpty() (QList does not cache size)

src/subscription/subscriptionlistmodel.h
75

Missing override

sagara marked 2 inline comments as done.Oct 1 2017, 9:38 PM
sagara updated this revision to Diff 20218.Oct 1 2017, 9:39 PM
  • Added missing override, using isEmpty instead of size check
sagara updated this revision to Diff 20220.Oct 1 2017, 9:53 PM
  • Fixed missing include
mlaurent requested changes to this revision.Oct 2 2017, 5:18 AM
mlaurent added a subscriber: mlaurent.
mlaurent added inline comments.
src/data/akregator_part.rc
19

missing update version in this file

src/selectioncontroller.h
111

move just after private: please

src/subscription/subscriptionlistmodel.h
56

move code in cpp file please

This revision now requires changes to proceed.Oct 2 2017, 5:18 AM
sagara updated this revision to Diff 20275.Oct 3 2017, 1:08 AM
sagara marked 3 inline comments as done.
  • Moved implementions out of header file
  • Updated akregator_part.rc version
mlaurent accepted this revision.Oct 3 2017, 4:42 AM

Seems ok for me.
Thabks

dvratil accepted this revision.Oct 3 2017, 10:14 AM
This revision is now accepted and ready to land.Oct 3 2017, 10:14 AM

Ping ? did you have problem for commiting it ? I can do it for you if you want.

Ahh I thought one of you had to do it after reading https://community.kde.org/Infrastructure/Phabricator#Workflow I guess all I need to do is arc land to squash all the commits together correct?

Since I don't have developer account I can't push changes to the repo myself, so I would need one of you to handle the final merge.

Ok I will do it.
Regards

This revision was automatically updated to reflect the committed changes.