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 618055 - Allow dragging an item from the dash to any workspace in the linear view
Allow dragging an item from the dash to any workspace in the linear view
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-07 20:35 UTC by Marina Zhurakhinskaya
Modified: 2010-05-27 10:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[linearView] Zoom out when dragging an item from the dash (22.49 KB, patch)
2010-05-08 15:35 UTC, Florian Müllner
none Details | Review
[linearView] Zoom out when dragging an item from the dash (8.11 KB, patch)
2010-05-08 22:45 UTC, Florian Müllner
reviewed Details | Review
[linearView] Zoom out when dragging an item from the dash (9.04 KB, patch)
2010-05-09 08:30 UTC, Florian Müllner
committed Details | Review

Description Marina Zhurakhinskaya 2010-05-07 20:35:50 UTC
Enable the same kind of zooming out and sliding along workspaces animation when dragging an item from the dash, as the animation that happens when a window is dragged to another workspace in the linear view. Offer a new workspace at the outer edge of the workspaces too.

We actually allow dropping dash items on the workspace indicators in the bottom of the overview, but that's not as intuitive or convenient.
Comment 1 Florian Müllner 2010-05-08 15:35:23 UTC
Created attachment 160585 [details] [review]
[linearView] Zoom out when dragging an item from the dash

When dragging windows in linear view, the workspace zooms out to
allow moving the window to other workspaces. Enable the same
behaviour for items dragged from the dash.
Comment 2 Owen Taylor 2010-05-08 19:42:22 UTC
Review of attachment 160585 [details] [review]:

This feels like a forest of signal connections to me. Wouldn't it be a lot simpler if 

  this._draggable.connect('drag-begin',
      Lang.bind(this, function() {
	this.emit('drag-begin');
  }));

Was:

 this._draggable.connect('drag-begin', 
                         function() { Main.overview.beginItemDrag(); });

?

(Or Lang.bind(Main.overview, Main.overview.beginItemDrag()), but that's probably harder to read.)
Comment 3 Florian Müllner 2010-05-08 22:45:25 UTC
Created attachment 160612 [details] [review]
[linearView] Zoom out when dragging an item from the dash

(In reply to comment #2)
> This feels like a forest of signal connections to me. Wouldn't it be a lot
> simpler if [...]

Yes!
Comment 4 Owen Taylor 2010-05-08 23:16:27 UTC
Review of attachment 160612 [details] [review]:

Much easier to review this way!

::: js/ui/genericDisplay.js
@@ +51,3 @@
+                          function() {
+                              Main.overview.endItemDrag();
+                          });

This needs a comment as to who is going to call beginItemDrag (genericDisplay) and in general, doesn't feel that secure to me that we won't get unbalanced calls. Is there a reason not to do the beginItemDrag here as well? If so, needs a comment.

::: js/ui/overview.js
@@ +414,1 @@
             || source instanceof AppDisplay.AppIcon) {

Hmm, I was going to complain about AppDisplay.AppIcon not calling endItemDrag, but do we still drag it at all any more or do we always drag an AppWellIcon?
Comment 5 Florian Müllner 2010-05-09 08:30:29 UTC
Created attachment 160624 [details] [review]
[linearView] Zoom out when dragging an item from the dash

(In reply to comment #4)
> ::: js/ui/genericDisplay.js
> @@ +51,3 @@
> +                          function() {
> +                              Main.overview.endItemDrag();
> +                          });
> 
> This needs a comment as to who is going to call beginItemDrag (genericDisplay)
> and in general, doesn't feel that secure to me that we won't get unbalanced
> calls. Is there a reason not to do the beginItemDrag here as well? If so, needs
> a comment.

Calling beginItemDrag() here mysteriously broke the closing of the moreDocs pane. The problem is as follows: When entering drag mode, the linear view raises an empty group actor spanning the whole monitor to the top - not sure if I understand it wholly, but one reason is the ability to have the screen edges reactive.

I now worked around it by closing the moreDocs pane explicitly - not too elegant as well, but still better ...

There is one problem remaining, which is loosing the ability to drag items from the allApps pane to the app well (in retrospect, the previous patch version had the same problem) - not sure if there's anything we can do about that ...


> ::: js/ui/overview.js
> @@ +414,1 @@
>              || source instanceof AppDisplay.AppIcon) {
> 
> Hmm, I was going to complain about AppDisplay.AppIcon not calling endItemDrag,
> but do we still drag it at all any more or do we always drag an AppWellIcon?

It's all AppWellIcon now.
Comment 6 Owen Taylor 2010-05-21 19:33:26 UTC
Review of attachment 160624 [details] [review]:

Not sure exactly what new code "I now worked around it by closing the moreDocs pane explicitly" refers to, but the patch looks generally good to me. Looks like you have the regressions in dragging to the side panel targeted in bug 619203, so I won't worry about them here.
Comment 7 Florian Müllner 2010-05-21 21:54:51 UTC
(In reply to comment #6)
> Not sure exactly what new code "I now worked around it by closing the moreDocs
> pane explicitly" refers to

There's a mechanism in dnd.js which allows objects to react to drag-over (more precise: if the actor beneath the dragged actor has a _delegate object with a handleDragOver method, the latter is executed).

It stopped working because of the linear view raising the monitor-sized transparent ClutterGroup above everything else ...

The patch actually removes the only use case for that hook, so I guess we could remove the entire mechanism.
Comment 8 Florian Müllner 2010-05-27 10:14:07 UTC
Attachment 160624 [details] pushed as a57a34c - [linearView] Zoom out when dragging an item from the dash