Review Request 126658: Notifications again not works correctly [revert parch 126408]

[Martin Klapetek]

> On Jan. 7, 2016, 4:36 a.m., Martin Klapetek wrote:
> > No.
> > 
> > I'm fine with disabling the first animation. Post a separate patch
> > for that and that only.
> > 
> > Any configuration issues that are left need to be identified
> > and fixed, not by reverting and moving things back again.
> 
> Anthony Fieroni wrote:
>     https://youtu.be/kavjfrRny9I
>     Video, flying with first test, see configuration.
>     Test mine, please, revert 126408.
> 
> Martin Klapetek wrote:
>     This is how much it takes to fix the config issue:
>     
>     @@ -39,8 +39,10 @@ QtControls.GroupBox {
>      
>          onEnabledChanged: {
>              if (!enabled) {
>     -            selectedPosition = NotificationsHelper.Default
>     +            positionRadios.current = null
>              }
>     +
>     +        selectedPosition = NotificationsHelper.Default
>          }
>      
>          PlasmaCore.Svg {
>     
>     
>     
>     ...without any reverting and probably faster than
>     it took you to make and upload the video.
>     
>     Again, I will accept only patch that disables the
>     first animation. There is no need for a revert.
> 
> Anthony Fieroni wrote:
>     Without video can't see it, first of all. I'm not providing a patch till this one is removed.

That's fine, I have a patch ready for that.


- Martin


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


On Jan. 7, 2016, 4:41 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126658/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2016, 4:41 a.m.)
> 
> 
> Review request for Plasma, David Edmundson, Marco Martin, and Martin Klapetek.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Review 126408 completely same flying and configurations issues
> 
> 
> Diffs
> -----
> 
>   applets/notifications/plugin/notificationshelper.cpp 15b4479 
>   applets/notifications/package/contents/ui/configNotifications.qml 95a8e59 
>   applets/notifications/package/contents/ui/Notifications.qml f479a65 
>   applets/notifications/lib/notificationsapplet.h 5b262f1 
>   applets/notifications/lib/notificationsapplet.cpp 891cdb0 
>   applets/notifications/plugin/notificationshelper.h 860a2da 
>   applets/notifications/package/contents/ui/ScreenPositionSelector.qml efff648 
> 
> Diff: https://git.reviewboard.kde.org/r/126658/diff/
> 
> 
> Testing
> -------
> 
> My code works *correctly*, revert patch from 126408 - configurations issues.
> Qt 5.5.1, libxcb 1.11.1 Arch Linux
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

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