Review Request 127120: Morphingpopups: Add config to disable size and fade animations

David Rosca nowrep at gmail.com
Mon Feb 22 10:41:35 UTC 2016



> On Feb. 22, 2016, 10:09 a.m., Marco Martin wrote:
> > -1
> > I'm against complicating the effect as well.
> > I don't want every time a small behavior change gets introduced, a config option popping up to "behave just like before". Especially because it won't.
> > If the size is not animated, more logic would needed to be reintroduced, that "anchoring you did put. that's complexity hidden behind a config option, meaning effectively dead code for most developers, the road to unmaintained :p
> > 
> > for me it's either having it or disabling animations completely by disabling the effect, especially because animate move without animating resizing was simply a wrong behavior, we just didn't have the means to do better.

> If the size is not animated, more logic would needed to be reintroduced, that "anchoring you did put.

I don't think so. That anchoring is mainly for tooltips in panels, and it works fine. Did you try it?

> for me it's either having it or disabling animations completely by disabling the effect, especially because animate move without animating resizing was simply a wrong behavior, we just didn't have the means to do better.

It may be wrong behavior, but it looked better overall imo.
The current animation looks great when there is only small difference between old/new tooltips, but also bad when the texture resizing is noticeable (when sizes/contents differ significantly).

As for disabling tooltips completely, that is now what you get with Plasma 5.5 and Frameworks 5.20.

Alright, I'll put it on kde-look.org instead.


> On Feb. 22, 2016, 10:09 a.m., Marco Martin wrote:
> > effects/morphingpopups/package/contents/code/morphingpopups.js, line 58
> > <https://git.reviewboard.kde.org/r/127120/diff/3/?file=444791#file444791line58>
> >
> >     removing this skip small steps probably makes sense

I'll open new rr for this.


- David


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


On Feb. 21, 2016, 9:31 a.m., David Rosca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127120/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2016, 9:31 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kwin
> 
> 
> Description
> -------
> 
> Disabling both animations makes the effect look like the old Plasma tooltip animation.
> Also don't animate when geometry didn't change + don't skip small steps as resize may happen in steps so we must animate all of them.
> 
> 
> Diffs
> -----
> 
>   effects/morphingpopups/package/contents/code/morphingpopups.js ce6ad86 
>   effects/morphingpopups/package/contents/config/main.xml PRE-CREATION 
>   effects/morphingpopups/package/contents/ui/config.ui PRE-CREATION 
>   effects/morphingpopups/package/metadata.desktop f9b192c 
> 
> Diff: https://git.reviewboard.kde.org/r/127120/diff/
> 
> 
> Testing
> -------
> 
> Changing config from KCM needs KWin restart because even though `configChanged` is emitted, `readConfig` returns the old values (bug, or am I doing something wrong?).
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


More information about the Plasma-devel mailing list