After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739641 - Emit animation-completed::zoom?
Emit animation-completed::zoom?
Status: RESOLVED FIXED
Product: libchamplain
Classification: Core
Component: view
unspecified
Other Linux
: Normal normal
: ---
Assigned To: libchamplain-maint
libchamplain-maint
Depends on:
Blocks:
 
 
Reported: 2014-11-04 20:00 UTC by Jonas Danielsson
Modified: 2014-11-10 06:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ChamplainView: Emit animation-completed::zoom (1.25 KB, patch)
2014-11-04 20:01 UTC, Jonas Danielsson
committed Details | Review

Description Jonas Danielsson 2014-11-04 20:00:36 UTC
Would it make sense to emit this signal on zoom animation complete?

The documentation states:
"This is a detailed signal.  For example, if you want to be signaled only for go-to animation, you should connect to "animation-completed::go-to"."

But it is only emitted for animation-completed::go-to as far as I can see.
Having would give application developers more fine grained control of when to do stuff. But maybe it will break stuff that assumes that animation-completed is only for go-to?
Comment 1 Jonas Danielsson 2014-11-04 20:01:01 UTC
Created attachment 289990 [details] [review]
ChamplainView: Emit animation-completed::zoom
Comment 2 Jonas Danielsson 2014-11-04 20:05:22 UTC
And maybe ensure_visible that potentially does both zoom and go-to should emit just animation-completed when both are done?
Comment 3 Jonas Danielsson 2014-11-04 20:07:08 UTC
(In reply to comment #2)
> And maybe ensure_visible that potentially does both zoom and go-to should emit
> just animation-completed when both are done?

Na, I guess that doesn't work.
Comment 4 Jiri Techet 2014-11-07 18:01:31 UTC
Review of attachment 289990 [details] [review]:

Looks good.
Comment 5 Jiri Techet 2014-11-07 18:04:44 UTC
Makes sense - when I added the zoom animation, I forgot we have some signal for animation completion and that I should add a signal for it. I've just pushed the patch to master, thanks.

Just wondering - is that something you just noticed or is there some real practical need for this signal?
Comment 6 Jonas Danielsson 2014-11-10 06:11:58 UTC
(In reply to comment #5)
> Makes sense - when I added the zoom animation, I forgot we have some signal for
> animation completion and that I should add a signal for it. I've just pushed
> the patch to master, thanks.
> 
> Just wondering - is that something you just noticed or is there some real
> practical need for this signal?

There have been sometimes during Maps dev that I felt like I wanted one. And I will add this to a few places in Maps.