Review Request 119916: Make the notification popup higher when 3 actions are present

Martin Klapetek martin.klapetek at gmail.com
Tue Sep 2 14:26:37 UTC 2014



> On Aug. 24, 2014, 11:55 a.m., Kai Uwe Broulik wrote:
> > applets/notifications/package/contents/ui/NotificationPopup.qml, line 60
> > <https://git.reviewboard.kde.org/r/119916/diff/1/?file=307345#file307345line60>
> >
> >     Why not make it take into account the button actual size? Such as Math.max(5 * units.gridUnit, totalHeightOfTheButtonsWithSpacing)

I've spent good half of the day on this and it turns out it's not as straightforward, here's why
* changing the popup content triggers the binding of the height property
* changing the height triggers the sync size to main item in Dialog
* all this takes some time
* at this point the popup->height() in the c++ side will return invalid height (the not-yet-changed) and will result in wrong positioning of all the popups

...which actually also breaks the original patch I believe, I just didn't notice. *sigh*


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119916/#review65112
-----------------------------------------------------------


On Aug. 24, 2014, 1:19 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119916/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2014, 1:19 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Since the port of the Button component to QQC Button, the button height cannot be set anymore, therefore the height of the popup now needs to be higher when there are three actions.
> 
> 
> Diffs
> -----
> 
>   applets/notifications/package/contents/ui/NotificationPopup.qml 489fbd1 
>   applets/notifications/plugin/notificationshelper.cpp 783c0e4 
> 
> Diff: https://git.reviewboard.kde.org/r/119916/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Screenshot
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/08/23/826a7f99-31f9-47e6-ba58-82ba523f5728__notifications-3actions.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


More information about the Plasma-devel mailing list