GNOME Bugzilla – Bug 685285
No animations when dragging to create a new workspace
Last modified: 2013-03-17 18:36:52 UTC
Dragging a window or an application launcher between two workspaces creates a new workspace. To indicate this, we increase the padding between the two workspaces and insert a spacer graphic when the dragged object is hovered over the space between workspaces. However, these transitions are not animated, nor is the insertion of the new workspace. The result is jerky and disharmonious, and it doesn't clearly convey the arrival of the new workspace. Two things we can do: * For the hover state, smoothly increase the space between the workspaces and make the spacer appear to grow into the space. * Animate in the new workspace.
Yeah, this was because I didn't have time to do this. It's a hard problem; the workspace thumbnails code is complex.
Created attachment 231680 [details] [review] WorkspaceThumbnails: animate the right workspace when creating by DND To create a new workspace by dropping on the placeholder, we move all the windows down one workspace and then wait for _checkWorkspaces() to automatically create the empty one at the end. This means that, from the implementation POV, the new workspace is not the one created by DND but it's the last one, and this detail was exposed in the UI because the animation was applied on that one. Fix that by starting the animation manually from the DND code, and then blocking the animation from happening when the new workspace is created with a flag.
Created attachment 231681 [details] [review] Workspace/WorkspaceThumbnail: don't destroy actors while accepting a drag The DND code treats a destroyed actor as a cancelled drag and emits the drag-end signal. That causes clutter warnings from signal handlers that try to reparent the destroyed actor. I forgot, the previous patch is on top of this one, otherwise it probably won't apply.
Review of attachment 231681 [details] [review]: So, what's happening is that the source actor is destroyed, which causes the clone to be destroyed as well? Wouldn't constructing a new WindowClone instead of using a ClutterClone for the DND actor work as well? ::: js/ui/workspace.js @@ +1992,1 @@ let index = this.metaWorkspace ? this.metaWorkspace.index() : global.screen.get_active_workspace_index(); At some point we gotta clean up the mutter codebase to be based around MetaWorkspace, not a hybrid of that and indexes. @@ +2005,3 @@ + false, // don't create workspace + time); + }); return false; @@ +2008,3 @@ + + // Tell the actor we're accepting the drop, but we wish to keep it alive + Main.uiGroup.remove_actor(actor); How does this work?
Comment on attachment 231681 [details] [review] Workspace/WorkspaceThumbnail: don't destroy actors while accepting a drag Ok, this patch is totally wrong. I have a better series, which addresses the real problems.
Created attachment 233818 [details] [review] Workspace/WorkspaceThumbnail: fix Clutter warnings from stacking code Windows can be restacked at any time, including when the stackAbove property of the window clone is during a drag, and thus parented to the uiGroup. To do stacking properly, we need to skip it for the duration of the drag, and sync it again at the end (which is already done by mutter because of the workspace change)
Created attachment 233819 [details] [review] DND: don't cancel a drag if the actor is destroyed inside acceptDrop This happens in the case of Workspace/WorkspaceThumbnail: they call meta_window_change_workspace_by_index(), which fires window-removed on the old workspace, thus destroying the window clone.
Created attachment 233820 [details] [review] WorkspacesView: don't fail when doing window DND from the last workspace DND of windows has a lot of side effects, including the possibility of current workspace disappering from under our feet. We need to account for that when trying to activate it.
Created attachment 233821 [details] [review] WorkspaceThumbnails: animate the right workspace when creating by DND To create a new workspace by dropping on the placeholder, we move all the windows down one workspace and then wait for _checkWorkspaces() to automatically create the empty one at the end. This means that, from the implementation POV, the new workspace is not the one created by DND but it's the last one, and this detail was exposed in the UI because the animation was applied on that one. Fix that by starting the animation manually from the DND code, and then blocking the animation from happening when the new workspace is created with a flag. Rebased (and maybe not even requiring all the stacking changes if one just wants to test the animation)
Review of attachment 233818 [details] [review]: "including when the stackAbove property of the window clone is during a drag" I think you're missing a word here.
Hard code freeze ping! (I'll commit the acn one before the release if you don't review the others)
Review of attachment 233821 [details] [review]: OK. ::: js/ui/workspaceThumbnail.js @@ +981,3 @@ this._indicator.raise_top(); + + // Clear the splice index, we got the message "the message"? What message?
Review of attachment 233820 [details] [review]: ::: js/ui/workspacesView.js @@ +417,3 @@ + + if (this._workspaces[current]) { + metaWorkspace = this._workspaces[current].metaWorkspace; if (!this._workspaces[current]) { // The current workspace was destroyed. // ... current = this._workspaces.length - 1; }
Review of attachment 233819 [details] [review]: OK.
Created attachment 239069 [details] [review] WorkspacesView: don't fail when doing window DND from the last workspace DND of windows has a lot of side effects, including the possibility of current workspace disappering from under our feet. We need to account for that when trying to activate it.
Review of attachment 239069 [details] [review]: ::: js/ui/workspacesView.js @@ +401,3 @@ + } + + metaWorkspace = this._workspaces[current].metaWorkspace; Uh, you can move the "let" back here now.
Attachment 233818 [details] pushed as ceae032 - Workspace/WorkspaceThumbnail: fix Clutter warnings from stacking code Attachment 233819 [details] pushed as 57f2757 - DND: don't cancel a drag if the actor is destroyed inside acceptDrop Attachment 233821 [details] pushed as ee0040e - WorkspaceThumbnails: animate the right workspace when creating by DND Attachment 239069 [details] pushed as 9e60a55 - WorkspacesView: don't fail when doing window DND from the last workspace