----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109825/#review30479 -----------------------------------------------------------
resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22718> I think QPointer::operator->() is implemented such that it returns the contained class's interface I.e. I think you can write mNotificationSNI->setCategory... looks nicer IMHO resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22719> Since the resource is a KApplication (see ResourceBase::init()) it should already have a KAboutData setup somewhere. Might be worth checking if you can use that one instead resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22720> Actually it might even be possible to access the KComponentData object of the resource directly, KGlobal::mainComponent() looks like it would deliver that resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22721> while you have a point that the case of n == 1 is already covered, my still limited understanding of internationalization is that some languages have more complex plural form rules. Better check with the translators (I can do that if you are not subscribed to their mailinglist) resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22722> I wonder if this should not be an assert resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22723> just curious: if this branch is triggered, where does sni get deleted then? resources/facebook/facebookresource_notifications.cpp <http://git.reviewboard.kde.org/r/109825/#comment22724> is this something you haven't gotten around to do yet or something you are not sure how to do it? - Kevin Krammer On April 5, 2013, 4:01 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109825/ > ----------------------------------------------------------- > > (Updated April 5, 2013, 4:01 p.m.) > > > Review request for KDEPIM and Plasma. > > > Description > ------- > > This patch displays a KNotification whenever an unread notification exists on > Facebook. This KNotification groups at most 3 notifications into one popup > and then say "...and N more" if there is more. It also keeps track of which > notifications were already displayed and does not display them again unless > they were updated on the server. This is all stored in a separate config > file. > > Then it creates a KSNI for the notifications where it always show the newest > three notifications in the tooltip (regardless if it was displayed before or > not) and creates a menu with the notifications, which opens browser with the > notification link. > > > Diffs > ----- > > resources/facebook/CMakeLists.txt e8c6381 > resources/facebook/facebookresource.h 4a16c0c > resources/facebook/facebookresource.cpp 67e8f3b > resources/facebook/facebookresource_notifications.cpp 7f6b8c4 > resources/facebook/serializer/akonadi_serializer_socialnotification.cpp > a261e14 > resources/facebook/settingsbase.kcfg 9f8e4b5 > resources/facebook/settingsdialog.cpp bfb7826 > resources/facebook/settingsdialog.ui 68b6a24 > > Diff: http://git.reviewboard.kde.org/r/109825/diff/ > > > Testing > ------- > > Yes. > > > File Attachments > ---------------- > > KSNI > > http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/facebook_notifications.png > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
