GNOME Bugzilla – Bug 637104
don't allow positioning before or after self in dnd
Last modified: 2011-01-06 13:31:54 UTC
It is pretty silly to allow dropping an icon immediately next to the icon that is being dragged. We shouldn't allow it.
Created attachment 177638 [details] [review] DashDND: Don't allow positioning before or after self Don't allow the icon to be dropped immediately next to itself.
Review of attachment 177638 [details] [review]: Overall looks good, some suggestions below: ::: js/ui/dash.js @@ +274,3 @@ return DND.DragMotionResult.NO_DROP; + let favoritesList = AppFavorites.getAppFavorites().getFavorites(); Not sure about the 'List' suffix for an array - why not just call the variable 'favorites'? @@ +277,3 @@ + let numFavorites = favoritesList.length; + + let appPos = -1; Easier to do let appPos = favoritesList.indexOf(app); Also, I think something like favPos is slightly better - it's a bit weird that the dash contains apps with an appPos of -1 ... @@ +302,3 @@ if (this._dragPlaceholder) this._dragPlaceholder.destroy(); + Whitespace on empty line
(In reply to comment #2) > Review of attachment 177638 [details] [review]: > > Overall looks good, some suggestions below: > > ::: js/ui/dash.js > @@ +274,3 @@ > return DND.DragMotionResult.NO_DROP; > > + let favoritesList = AppFavorites.getAppFavorites().getFavorites(); > > Not sure about the 'List' suffix for an array - why not just call the variable > 'favorites'? To not conflict with the 'favorites' used by for the map later in the function ... but I ended up removing it anyway. > @@ +277,3 @@ > + let numFavorites = favoritesList.length; > + > + let appPos = -1; > > Easier to do > > let appPos = favoritesList.indexOf(app); > > Also, I think something like favPos is slightly better - it's a bit weird that > the dash contains apps with an appPos of -1 ... OK
Created attachment 177650 [details] [review] DashDND: Don't allow positioning before or after self Don't allow the icon to be dropped immediately next to itself.
Review of attachment 177650 [details] [review]: Looks good.
Attachment 177650 [details] pushed as cff5039 - DashDND: Don't allow positioning before or after self