----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120943/#review69715 -----------------------------------------------------------
Heh, looking at the code again on my way in the office I saw a few weird things. And some questions to the original authors :) applets/notifications/package/contents/ui/NotificationDelegate.qml <https://git.reviewboard.kde.org/r/120943/#comment48829> Just in case you were wondering: If you have a JS Array in a ListModel role, that thing is automatically converted into another ListModel :) applets/notifications/package/contents/ui/NotificationDelegate.qml <https://git.reviewboard.kde.org/r/120943/#comment48830> Do we really need that? applets/notifications/package/contents/ui/NotificationPopup.qml <https://git.reviewboard.kde.org/r/120943/#comment48827> Actually we could just bind those to notificationProperties.foo instead of assigning everything manually applets/notifications/package/contents/ui/NotificationPopup.qml <https://git.reviewboard.kde.org/r/120943/#comment48828> I think that is not needed applets/notifications/package/contents/ui/NotificationPopup.qml <https://git.reviewboard.kde.org/r/120943/#comment48826> Whoops. applets/notifications/package/contents/ui/NotificationPopup.qml <https://git.reviewboard.kde.org/r/120943/#comment48831> Somehow I now no longer get line breaks in KMail notifications. But StyledText does support <br>, <b> and stuff - Kai Uwe Broulik On Nov. 2, 2014, 8 nachm., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120943/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2014, 8 nachm.) > > > Review request for Plasma and Martin Klapetek. > > > Repository: plasma-workspace > > > Description > ------- > > This moves duplicate code (notification icon, heading, text, action buttons) > into a separate NotificationItem {} component used by both the notification > popup as well as the notification list delegate. > > If we ever were to provide richer notifications for specific usecases (you > can provide custom hints in notifications after all) this would ease this and > less duplication is always good :) > > I also cleaned up a bit of commented/unused code and changed complicated > anchoring to QtQuick Layouts where applicable. > > > Diffs > ----- > > applets/notifications/package/contents/ui/NotificationDelegate.qml 88f6cd2 > applets/notifications/package/contents/ui/NotificationItem.qml PRE-CREATION > applets/notifications/package/contents/ui/NotificationPopup.qml 26c7ed2 > > Diff: https://git.reviewboard.kde.org/r/120943/diff/ > > > Testing > ------- > > Ran knotificationdbustest and didn't notice anything unusual, visually the > thing should look and behave exactly like it did before. Additional testing > is welcomed, however. > > > File Attachments > ---------------- > > New popup > > https://git.reviewboard.kde.org/media/uploaded/files/2014/11/02/9b20416e-a4c6-4694-aff3-bdad26cf3206__newnotificationpopup.png > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
