> On Nov. 3, 2014, 10:18 a.m., Kai Uwe Broulik wrote: > > applets/notifications/package/contents/ui/NotificationDelegate.qml, lines > > 273-278 > > <https://git.reviewboard.kde.org/r/120943/diff/2/?file=324533#file324533line273> > > > > Do we really need that?
I'm really not sure, I can look into this afterwards if you want > On Nov. 3, 2014, 10:18 a.m., Kai Uwe Broulik wrote: > > applets/notifications/package/contents/ui/NotificationPopup.qml, lines 48-51 > > <https://git.reviewboard.kde.org/r/120943/diff/2/?file=324535#file324535line48> > > > > Actually we could just bind those to notificationProperties.foo instead > > of assigning everything manually Ah yeah, that's something I was thinking about doing some time ago, I can do that as well if you don't want to > On Nov. 3, 2014, 10:18 a.m., Kai Uwe Broulik wrote: > > applets/notifications/package/contents/ui/NotificationPopup.qml, line 54 > > <https://git.reviewboard.kde.org/r/120943/diff/2/?file=324535#file324535line54> > > > > I think that is not needed Wouldn't it just keep appending then? - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120943/#review69715 ----------------------------------------------------------- On Nov. 2, 2014, 9 p.m., 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, 9 p.m.) > > > 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
