----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120943/#review69738 -----------------------------------------------------------
I think it's a good approach and quite possibly worth pursuing, cheers ^_- Can you also post a screenshot of the history popup please? File Attachment: New popup - newnotificationpopup.png <https://git.reviewboard.kde.org//r/120943/#fcomment272> The icon was always aligned to the top, together with the title. I'm not sure this looks better...add vdg? applets/notifications/package/contents/ui/NotificationDelegate.qml <https://git.reviewboard.kde.org/r/120943/#comment48844> The plasma qml style[1] says all properties need to be on top; makes it also better readable [1] https://community.kde.org/Plasma/QMLStyle applets/notifications/package/contents/ui/NotificationItem.qml <https://git.reviewboard.kde.org/r/120943/#comment48845> I know this is just copied but I wonder - is there any reason for this? I mean do notifications have urls in the title? (question to whoever put it there originally) - Martin Klapetek 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
