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 690175 - Refactor WorkspaceThumbnail to be independent from WorkspacesDisplay
Refactor WorkspaceThumbnail to be independent from WorkspacesDisplay
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks: 682050
 
 
Reported: 2012-12-13 19:40 UTC by Cosimo Cecchi
Modified: 2012-12-19 10:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
workspaceThumbnail: make ThumbnailsBox track workspace changes itself (6.19 KB, patch)
2012-12-13 19:40 UTC, Cosimo Cecchi
committed Details | Review
workspaceThumbnail: handle scroll-event internally (2.84 KB, patch)
2012-12-13 19:40 UTC, Cosimo Cecchi
committed Details | Review
workspaceThumbnail: handle entering the overview internally (2.49 KB, patch)
2012-12-13 19:40 UTC, Cosimo Cecchi
committed Details | Review
workspaceThumbnail: unify restack handling (5.08 KB, patch)
2012-12-13 19:40 UTC, Cosimo Cecchi
needs-work Details | Review
workspaceThumbnail: unify restack handling (5.08 KB, patch)
2012-12-17 11:20 UTC, Cosimo Cecchi
committed Details | Review

Description Cosimo Cecchi 2012-12-13 19:40:11 UTC
The final goal is to decouple completely WorkspaceThumbnail from WorkspacesDisplay, and add it directly to the overview's box layout; these are a few preparatory patches that will help us getting there.
Comment 1 Cosimo Cecchi 2012-12-13 19:40:14 UTC
Created attachment 231504 [details] [review]
workspaceThumbnail: make ThumbnailsBox track workspace changes itself

Instead of having WorkspacesDisplay calling us.

https://bugzilla.gnome.org/show_bug.cgi?id=682050
Comment 2 Cosimo Cecchi 2012-12-13 19:40:16 UTC
Created attachment 231505 [details] [review]
workspaceThumbnail: handle scroll-event internally

Instead of having WorkspacesDisplay calling us.

https://bugzilla.gnome.org/show_bug.cgi?id=682050
Comment 3 Cosimo Cecchi 2012-12-13 19:40:19 UTC
Created attachment 231506 [details] [review]
workspaceThumbnail: handle entering the overview internally

Instead of having WorkspacesDisplay calling us.
Comment 4 Cosimo Cecchi 2012-12-13 19:40:22 UTC
Created attachment 231507 [details] [review]
workspaceThumbnail: unify restack handling

Both WorkspacesDisplay and ThumbnailsBox need to know when windows have been
restacked. Instead of each tracking changes on their own or trying to call
each other, have the overview keep track and do the calculations, emitting
a signal with the result.
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-12-17 11:09:17 UTC
Review of attachment 231504 [details] [review]:

OK.
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-12-17 11:11:04 UTC
Review of attachment 231505 [details] [review]:

OK.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-12-17 11:12:52 UTC
Review of attachment 231506 [details] [review]:

Commit message needs work. Try something like:

    workspaceThumbnail: handle showing/hiding when entering/hiding the overview ourselves

    Instead of having WorkspacesDisplay show/hide us.
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-12-17 11:14:22 UTC
Review of attachment 231507 [details] [review]:

::: js/ui/overview.js
@@ +395,3 @@
+        }
+
+        this.emit('sync-window-stacking', stackIndices);

'windows-restacked' or something. It should be the past tense of a verb if we're not talking about a vfunc signal where we can *change* the synchronization of the window stacking.
Comment 9 Cosimo Cecchi 2012-12-17 11:20:15 UTC
Created attachment 231711 [details] [review]
workspaceThumbnail: unify restack handling

--

Use a better name for the signal
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-12-17 12:16:46 UTC
Review of attachment 231711 [details] [review]:

OK.
Comment 11 Cosimo Cecchi 2012-12-17 12:26:07 UTC
Pushed, thanks.