Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-06 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/ --- (Updated April 6, 2015, 6:09 p.m.) Status -- This change has been ma

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread Thomas Pfeiffer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review78385 --- Thank you for adding usability! If the checkbox is disabled,

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread David Edmundson
> On April 1, 2015, 4:48 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.h, line 35 > > > > > > If you already have WRITE and NOTIFY, just add a READ too MEMBER creates an implic

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread Marco Martin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review78362 --- Ship it! In principle I would prefer setting the position by

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review78360 --- Ship it! Some really minor nit picky things below, feel free

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread Martin Gräßlin
> On April 1, 2015, 4:45 p.m., Martin Klapetek wrote: > > Hello? C++ code looks good to me, QML needs to be reviewed by someone else - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.or

Re: Review Request 122842: Allow selecting notifications position on screen

2015-04-01 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review78341 --- Hello? - Martin Klapetek On March 24, 2015, 1:19 p.m., Mart

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Martin Klapetek
> On March 24, 2015, 5:22 p.m., Kai Uwe Broulik wrote: > > Nice! > > > > I'm not sure whether a global setting should be in the applet configuration > > but fair enough. The "Default" keeps the previous behavior where it sort-of > > follows your panel? Yes. > On March 24, 2015, 5:22 p.m., K

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review78005 --- Nice! I'm not sure whether a global setting should be in the

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Martin Klapetek
> On March 24, 2015, 1:33 p.m., Aleix Pol Gonzalez wrote: > > File Attachment: New Screenshot - notifications_config.png > > > > > > We usually have a title over here. > > Martin Klapetek wrote: > Ah yeah, I thought there's somethin

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Martin Klapetek
> On March 24, 2015, 1:33 p.m., Aleix Pol Gonzalez wrote: > > File Attachment: New Screenshot - notifications_config.png > > > > > > We usually have a title over here. Ah yeah, I thought there's something missing. That should be a diffe

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review77996 --- File Attachment: New Screenshot - notifications_config.png

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-24 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/ --- (Updated March 24, 2015, 1:19 p.m.) Review request for Plasma. Changes

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-12 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/ --- (Updated March 11, 2015, 7:09 p.m.) Review request for Plasma. Changes

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-11 Thread Kai Uwe Broulik
> On März 11, 2015, 6:46 nachm., Kai Uwe Broulik wrote: > > applets/notifications/package/contents/ui/ScreenPositionSelector.qml, line > > 37 > > > > > > Default init it to [] in case somebody forgets to assign t

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-11 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review77312 --- applets/notifications/package/contents/ui/ScreenPositionSelec

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-11 Thread Martin Klapetek
> On March 11, 2015, 7:06 p.m., Kai Uwe Broulik wrote: > > PlasmaExtra components maybe? I'm still unsure about the notifications on a > > per-applet basis :/ We could make them share the config? - Martin --- This is an automatically g

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-11 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review77310 --- PlasmaExtra components maybe? I'm still unsure about the notif

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-11 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/ --- (Updated March 11, 2015, 6:55 p.m.) Review request for Plasma. Changes

Re: Review Request 122842: Allow selecting notifications position on screen

2015-03-06 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122842/#review77114 --- File Attachment: Screenshot - notification_pos.png