Created attachment 339778 [details] [review]
overview: Application icon list doesn't disappear

When you login system first time after installing system, click
'Activities' to show overview screen, check 'Show Applications' button
to show application icon list, then uncheck 'Show Applications' button,
application icon list doesn't disappear.

In BaseAppView.animate function, code emit signal 'animation-done'
first in _doSpringAnimation, and then connect signal 'animation-done',
that cause application icon list can't hide.
We change code to connect signal first and then emit.

Review of attachment 339778 [details] [review]:

(In reply to xiaoguang wang from comment #1)
> In BaseAppView.animate function, code emit signal 'animation-done'
> first in _doSpringAnimation

Mmh, this should only be the case when there are no actors to animate, or when addTween() runs the onComplete function immediately. The former shouldn't show the symptom you are fixing, so I'll assume the latter, but it's not clear why this would happen. Do you have any insights there? Also do the icons animate out properly with the patch applied, or do they just disappear?

(In reply to Florian Müllner from comment #2)

> Mmh, this should only be the case when there are no actors to animate, or
> when addTween() runs the onComplete function immediately. The former
> shouldn't show the symptom you are fixing, so I'll assume the latter, but
> it's not clear why this would happen. Do you have any insights there? Also
> do the icons animate out properly with the patch applied, or do they just
> disappear?

More infromation, when you click 'Show Applications' more times, the problem will disappear.

When 'Show Applications' button unchecked, the function call flow is:
viewSelector._onShowAppsButtonToggled
viewSelector._showPage
viewSelector._animateOut
            this.appDisplay.animate(IconGrid.AnimationDirection.OUT, Lang.bind(this,
                function() {
                    this._animateIn(oldPage)
                }));
appDisplay.animate
	let currentView = this._views[global.settings.get_uint('app-picker-view')].view;
        currentView.animate(animationDirection, onComplete);
BaseAppView.animate
        this._doSpringAnimation(animationDirection);
        if (onComplete) {
            let animationDoneId = this._grid.connect('animation-done', Lang.bind(this,
                function () {
                    this._grid.disconnect(animationDoneId);
                    onComplete();
BaseAppView._doSpringAnimation
IconGrid.animateSpring
        if (actors.length == 0) {
            this._animationDone();
            return;
        }
IconGrid._animationDone
	this._animating = false;
	this.emit('animation-done');

From call flow we can see, when unchecking 'Show Applications' button first time, signal 'animation-done' don't connect to appDisplay._animateIn(), so this time, emitting 'animation-done' will not call appDisplay._animateIn().
But when you unchecking 'Show Applications' button second time, emitting 'animation-done' will call appDisplay._animateIn(), because first time 'animation-done' is connected to appDisplay._animateIn(). That is why we click 'Show Applications' button more times, problem disappears.

FrequentView.animate use BaseAppView.animate. AllView.animate override

(In reply to xiaoguang wang from comment #3)
> 
> FrequentView.animate use BaseAppView.animate. AllView.animate override

FrequentView.animate use BaseAppView.animate. AllView.animate override BaseAppView.animate.

(In reply to xiaoguang wang from comment #4)
> (In reply to xiaoguang wang from comment #3)
> > 
> > FrequentView.animate use BaseAppView.animate. AllView.animate override
> 
> FrequentView.animate use BaseAppView.animate. AllView.animate override
> BaseAppView.animate.

The patch has been tested on 3.20 and 3.22, work properly, problem doesn't happen.

More information:

New easy way to reproduce problem:
1. Click 'Activities' button, show overview screen.
2. Check 'Show Applications' button, show all application icon list.
3. Click 'Frequent' button.
4. Restart computer now.

After system starts up, following process in description you can see the problem.

(In reply to xiaoguang wang from comment #5)
> The patch has been tested on 3.20 and 3.22, work properly, problem doesn't
> happen.

No, it doesn't. It does fix one issue, but in case of the issue mentioned in this bug - the application icons not being hidden when unselecting the show-apps button - it just papers over a different issue.

Created attachment 340138 [details] [review]
appDisplay: Use correct view for animation

We currently assume that the current view matches the 'app-picker-view'
setting. While that is usually the case, there is one notable exception:
While there isn't sufficient usage data (yet), we show all applications
instead of an empty frequent view regardless of the setting. We should
animate the actually visible icons in that case, not the (non-existent)
ones from the hidden frequent view.

Review of attachment 339778 [details] [review]:

So while this patch isn't correct for the issue you describe - the bug there is that we hit the 0-icon case when there clearly are visible icons - it is correct that we need to handle an empty view. Not having any frequent applications is an expected case, and while we don't have any icons to animate out, we still need to hide the placeholder label. So the code is a correct fix for *that* issue, but please clarify the commit message.

Created attachment 340224 [details] [review]
appDisplay: Hide placeholder label with empty view

Click 'Activities' to show overview screen, check 'Show Applications',
then click 'Frequent' button, frequent view is empty and one line label
appears. Then uncheck 'Show Applications' button, label doesn't
disappear, and workspace view doesn't show.

In BaseAppView.animate function, code emit signal 'animation-done'
first in _doSpringAnimation, and then connect signal 'animation-done',
that cause placeholder label can't hide.

Possible duplicate of bug 742908.

Created attachment 340465 [details] [review]
appDisplay: Fix completion handler for empty animations

If an onComplete handler is passed to animate(), it is set to run at
the end of the animation via the icon grid's ::animation-done signal.
Currently the signal is connected after starting the animation, with
the result that the handler doesn't run when the animation completes
immediately (because there are no icons to animate). Fix this by only
starting the animation after connecting the signal.

Reuploading patch with clarified commit message.

Comment on attachment 340465 [details] [review]
appDisplay: Fix completion handler for empty animations

Code looks still good to me, so if you agree with the updated commit message, consider the patch status ACN.

Review of attachment 340465 [details] [review]:

Accept commit message.

Review of attachment 340138 [details] [review]:

looks fine

::: js/ui/appDisplay.js
@@ +1012,2 @@
     animate: function(animationDirection, onComplete) {
+        let currentView = this._views.filter(v => v.control.has_style_pseudo_class('checked')).pop().view;

we could have a this._currentView to keep this state but this works as well and the array only has 2 items *shrug*

Attachment 340138 [details] pushed as e661d90 - appDisplay: Use correct view for animation
Attachment 340465 [details] pushed as 09e6bb5 - appDisplay: Fix completion handler for empty animations

*** Bug 742908 has been marked as a duplicate of this bug. ***