Review Request 108325: LastNotificationPopup fix for closing not to move + add move button + better layout for 3 action buttons

[Albert Astals Cid]

> On April 5, 2013, 4:16 p.m., Albert Astals Cid wrote:
> > Can you please attach the diff properly and not give ship its to yourself?
> > 
> > I'd suggest you discard this review that is already broken and start a new one where you attach the diff properly and you don't give you a ship it to yourself and then maybe others will have a look at it.
> 
> Leszek Lesner wrote:
>     If I would know hot to attach the diff properly I would have done it. It is just not recognizing the patch.
> 
> Albert Astals Cid wrote:
>     What error are you getting? How are you creating the patch?
> 
> Leszek Lesner wrote:
>     I got the error that this is not a patch or that the specified diff file is empty.
>     I created the patch with dpkg-source --commit and removed the description lines above.
> 
> Albert Astals Cid wrote:
>     dpkg-source --commit seems like a bad way to create a git compatible diff.
>     
>     Just do "git diff" on the git repo.
> 
> Leszek Lesner wrote:
>     Hmm... I did not have any git repo there. I just downloaded the sources directly. Maybe that was the whole mistake. 
>     But it is some month already ago that I created that patch and it should be mostly outdated now that we have 4.10.2 out. 
>     All in all I am a little bit unhappy that I posted this patch on the bug tracker and no one really cared about it and then posting it here only on someones advice and it was still not reviewed or integrated. 
>     And I can't imagine that it is all my fault.
>
> 
> Marco Martin wrote:
>     you should download it with git, and send the patch against master branch, see http://techbase.kde.org/Getting_Started and  http://www.youtube.com/watch?v=cqnNVmJocR4

I'm discarding this review, you gave a ship it to yourself and that was wrong. I gave you the pointers on how to do it right, i'm sorry if you find it hard.


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108325/#review30476
-----------------------------------------------------------


On Jan. 10, 2013, 3:18 p.m., Leszek Lesner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108325/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2013, 3:18 p.m.)
> 
> 
> Review request for Plasma and Marco Martin.
> 
> 
> Description
> -------
> 
> Fixes the LastNotificationsPopup moving around when clicking on a button and adds a move button to freely place it all over the desktop. 
> Also this removes notifications jobs from the notification icon. (So clicking on X does not hide but really closes the notification and removes it)
> Also this fixes layout problems with 3 buttons displayed inside a notification (this caused the X button to overlap with the upper actionbutton).
> 
> As my original diff was not recognized as normal diff(don't know what I did wrong there) I attached the patch directly to this thread. 
> 
> 
> This addresses bug 311413.
>     http://bugs.kde.org/show_bug.cgi?id=311413
> 
> 
> Diffs
> -----
> 
> 
> Diff: http://git.reviewboard.kde.org/r/108325/diff/
> 
> 
> Testing
> -------
> 
> kdialog --passivepopup "bla"
> click on X, closes and removes job from notification Icon
> click and hold on move button moves the popup freely on the screen. 
> 
> 
> File Attachments
> ----------------
> 
> Patch
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/01/10/LastNotificationPopup_patch.diff
> 
> 
> Thanks,
> 
> Leszek Lesner
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130815/94408197/attachment-0001.html>