nicolasfella updated this revision to Diff 83322.
nicolasfella marked 8 inline comments as done.
nicolasfella added a comment.
- Address comments
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26918?vs=78423=83322
BRANCH
arcpatch-D26918
REVISION DETAIL
cblack requested changes to this revision.
cblack added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> Mainpage.dox:13
> +In order to perform a notification, you need to create a description file,
> which contains
> +default parameters of the notification. It
nicolasfella added a comment.
ping?
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To: nicolasfella, #frameworks, broulik, jucato
Cc: class, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham,
bruns
nicolasfella updated this revision to Diff 78423.
nicolasfella added a comment.
- Drop user config file description since it's an implementation detail
REPOSITORY
R289 KNotifications
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26918?vs=78416=78423
BRANCH
docs
REVISION
nicolasfella updated this revision to Diff 78416.
nicolasfella added a comment.
- Add a dedicated main page and link to the HIG from there
- Move most content to main page
- Fix code example
REPOSITORY
R289 KNotifications
CHANGES SINCE LAST UPDATE
nicolasfella added inline comments.
INLINE COMMENTS
> apol wrote in Mainpage.dox:9
> @class so we create a link.
Doxygen should do that automatically
http://www.doxygen.nl/manual/autolink.html
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To:
apol added inline comments.
INLINE COMMENTS
> broulik wrote in knotification.h:96
> It actually sends out the icon name as a string and plasmashell then looks it
> up.
With QIcon?
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To: nicolasfella,
broulik added inline comments.
INLINE COMMENTS
> apol wrote in knotification.h:96
> Since Qt5 KIconLoader is an implementation detail. Even our apps and
> frameworks use QIcon::fromTheme, I haven't looked at what KNotifications does
> but pretty sure it's using QIcon.
It actually sends out
apol added inline comments.
INLINE COMMENTS
> jucato wrote in knotification.h:96
> Maybe we should keep this part that the icon name, especially that it has to
> be one that can be found by KIconLoader.
Since Qt5 KIconLoader is an implementation detail. Even our apps and frameworks
use
jucato added inline comments.
INLINE COMMENTS
> knotification.h:96
> - *
> - * The icon filename is just the name, without extension, it's found with
> the KIconLoader.
> - * The Comment field will be used in KControl to describe the application.
Maybe we should keep this part that the
apol added a subscriber: class.
apol added a comment.
Let's do this. +1
INLINE COMMENTS
> Mainpage.dox:9
> +
> +KNotification is the main entry point for using KNotifications.
> +
@class so we create a link.
REPOSITORY
R289 KNotifications
REVISION DETAIL
nicolasfella added a task: T12709: Improvements for KNotifications
documentation/guidelines.
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To: nicolasfella, #frameworks, broulik, jucato
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh,
nicolasfella updated this revision to Diff 75799.
nicolasfella added a comment.
- Move most content to main page
REPOSITORY
R289 KNotifications
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26918?vs=75795=75799
BRANCH
docs
REVISION DETAIL
https://phabricator.kde.org/D26918
nicolasfella added inline comments.
INLINE COMMENTS
> apol wrote in knotification.h:58
> It could make sense to specify the cmake syntax to do that?
>
> `install(FILES appname.notifyrc DESTINATION ${KNOTIFYRC_INSTALL_DIR})`
Don't we have that already below?
REPOSITORY
R289 KNotifications
nicolasfella updated this revision to Diff 75795.
nicolasfella added a comment.
O - Add a dedicated main page and link to the HIG from there
REPOSITORY
R289 KNotifications
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26918?vs=74362=75795
BRANCH
docs
REVISION DETAIL
nicolasfella added inline comments.
INLINE COMMENTS
> apol wrote in knotification.h:44
> This entirely changes the semantics. Might still be correct but it could make
> sense to make sure that's the case?
> At least, for an action the user triggers should still be a feedback event,
> right?
I
apol added a comment.
+1 on improving the documentation, it does read dated. Thanks!
INLINE COMMENTS
> knotification.h:44
> * @li Feedback events:
> - * For notifying the user that he/she just performed an operation, like
> maximizing a
> - * window. This allows us to play sounds when a
nicolasfella added a comment.
Ping?
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To: nicolasfella, #frameworks, broulik, jucato
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
nicolasfella added a reviewer: jucato.
REPOSITORY
R289 KNotifications
REVISION DETAIL
https://phabricator.kde.org/D26918
To: nicolasfella, #frameworks, broulik, jucato
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
nicolasfella created this revision.
nicolasfella added reviewers: Frameworks, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
nicolasfella requested review of this revision.
REVISION SUMMARY
Various improvements. Better examples, updated coding
20 matches
Mail list logo
