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 722196 - viewSelector: Move to a sync() model for deciding which page to show
viewSelector: Move to a sync() model for deciding which page to show
Status: RESOLVED INCOMPLETE
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-14 16:16 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2019-02-27 20:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
viewSelector: Move to a sync() model for deciding which page to show (5.25 KB, patch)
2014-01-14 16:16 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2014-01-14 16:16:06 UTC
See patch. This is part of my overview transition cleanups.
Comment 1 Jasper St. Pierre (not reading bugmail) 2014-01-14 16:16:08 UTC
Created attachment 266276 [details] [review]
viewSelector: Move to a sync() model for deciding which page to show
Comment 2 Giovanni Campagna 2014-01-14 17:37:06 UTC
Review of attachment 266276 [details] [review]:

I don't really get what this is trying to fix, this is working code, and you're not really untangling it.

::: js/ui/viewSelector.js
@@ +119,3 @@
         Main.overview.connect('showing', Lang.bind(this,
             function () {
+                this._showAppsButton.checked = false;

How can showAppsButton.checked be true at this point?

@@ +125,3 @@
         Main.overview.connect('hiding', Lang.bind(this,
             function () {
+                this._showAppsButton.checked = false;

Doesn't this trigger a transition from apps to windows while closing?
Comment 3 Jasper St. Pierre (not reading bugmail) 2014-01-14 20:32:17 UTC
(In reply to comment #2)
> I don't really get what this is trying to fix, this is working code, and you're
> not really untangling it.

It's part of my work to fix the overview transition when launching an application. I figured this was worth a cleanup as it is. I agree that it's still quite messy though.

Entire branch is (WIP) at:

https://git.gnome.org/browse/gnome-shell/commit/?h=wip/overview-transition

> ::: js/ui/viewSelector.js
> @@ +119,3 @@
> How can showAppsButton.checked be true at this point?

I don't know, but resetShowAppsButton was called here before, and I remember something breaking if I didn't do this.

> @@ +125,3 @@
> Doesn't this trigger a transition from apps to windows while closing?

Yes, but that's already what happens. That's what the overall branch is trying to fix.
Comment 4 Giovanni Campagna 2014-01-14 20:55:40 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > I don't really get what this is trying to fix, this is working code, and you're
> > not really untangling it.
> 
> It's part of my work to fix the overview transition when launching an
> application. I figured this was worth a cleanup as it is. I agree that it's
> still quite messy though.
> 
> Entire branch is (WIP) at:
> 
> https://git.gnome.org/browse/gnome-shell/commit/?h=wip/overview-transition

WIP indeed - the stacking order in https://git.gnome.org/browse/gnome-shell/commit/?h=wip/overview-transition&id=ac1d5ba00c0e02ae1c25b0ddaf37f44fa011b9fa is wrong, because it puts top_window_group below the panel

> > ::: js/ui/viewSelector.js
> > @@ +119,3 @@
> > How can showAppsButton.checked be true at this point?
> 
> I don't know, but resetShowAppsButton was called here before, and I remember
> something breaking if I didn't do this.
> 
> > @@ +125,3 @@
> > Doesn't this trigger a transition from apps to windows while closing?
> 
> Yes, but that's already what happens. That's what the overall branch is trying
> to fix.

This is not the case, windows appear for a different reason (if you look at the workspace switcher, it's not visible when you exit the overview). Indeed, your last patch in the branch reintroduces showAppsBlocked for this.
Comment 5 Florian Müllner 2015-02-27 16:40:10 UTC
Any plans for picking this up at some point? Carlos did clean up the overview transition last summer, so I suppose the original motivation for the branch is obsolete?
Comment 6 Florian Müllner 2019-02-27 20:00:14 UTC
Closing this bug report as no further information has been provided. Please feel free to reopen this bug report if you can provide the information that was asked for in a previous comment.
Thanks!