Add 'Hello, Drag & Drop' sample application

[vin1antme]

Hello,

Here is a sample application "Hello, Drag & Drop" created according to TODO list.

The application implements dragging of text data between widgets, e. g. widgets involved in drag and drop should have text or textvariable attributes defined.

Also it adds draggable and droppable keywords to glimmer-dsl-tk. These keywords do not need configuration. Just add draggable to source and droppable to target widgets and drag with right-mouse button.

Would such implementation be appropriate for sample application or you had something other in mind?

[AndyObtiva]

Thank you for the effort. It's greatly appreciated.

I tried it and it didn't work on the Mac (dragging did nothing).

In any case, here is some feedback before your work is accepted:

• My Glimmer Style Guide does not encourage the style of setting a property on a widget without passing a value. In other words, it favors something like drag_source true (or draggable true) to configure a property for a pre-existing feature (instead of dynamically mixing in that feature into the widget)
• For drop, I do not agree with the default behavior you baked in. I prefer to make it more customizable via a listener and leave the details to the consumer. In other words, you would support something like on_drop {|event| ... }, and this automatically makes a destination widget droppable while offering the flexibility of doing anything, including deleting the dragged widget if needed.
• Also, offer support for all drag event listeners directly in the DSL if needed like on_drag_start and on_drag_motion

I look forward to seeing revised changes by you before accepting into the Glimmer DSL for Tk codebase.

By the way, thankfully I finished work on Glimmer DSL for LibUI, so I have an opening of time to focus on Glimmer DSL for Tk now.

Cheers.

[AndyObtiva]

BTW, you may also inspire your work by these Drag and Drop examples in Glimmer DSL for SWT:

• Drag and Drop for Widgets: https://github.com/AndyObtiva/glimmer-dsl-swt/blob/master/samples/hello/hello_drag_and_drop.rb
• Drag and Drop for Canvas Shapes: https://github.com/AndyObtiva/glimmer-dsl-swt/blob/master/samples/hello/hello_canvas_drag_and_drop.rb

[AndyObtiva]

One more thing I noticed.

You are defining model logic in an expression file. All model logic must live in models under lib/glimmer/tk

Also, you must have copied a static expression's code to build the drag and drop expression, but your drag and drop expression is dynamic since it supports multiple keywords dynamically based on logic. Static expressions do not usually need the "can_interpret?" method implementation as the superclass StaticExpression includes one by default that uses the name of the expression as a matcher (e.g. draggable for DraggableExpression) and pre-optimizes it by building a Glimmer method for it instead of passing the expression through the chain of all DSL expressions when checking against a keyword.

In other words, instead of this:

class DragAndDropExpression < StaticExpression
  def can_interpret?(parent, keyword, *args, &block)
    (keyword == "draggable" || keyword == "droppable") && args.size == 0
  end

You should have two static expressions for two keywords (optionally including can_interpret? if extra conditions need to be checked in addition to the super implementation):

class DraggableExpression < StaticExpression
  def can_interpret?(parent, keyword, *args, &block)
    super && args.size == 0 # super relies on default name matching
  end
end

and

class DroppableExpression < StaticExpression
  def can_interpret?(parent, keyword, *args, &block)
    super && args.size == 0 # super relies on default name matching
  end
end

Alternatively if you want to keep the expression dynamic, extend Expression instead and keep the can_interpret? method the way it was:

class DragAndDropExpression < Expression
  def can_interpret?(parent, keyword, *args, &block)
    (keyword == "draggable" || keyword == "droppable") && args.size == 0
  end
end

However, you probably should forget everything I mentioned above because if you implement draggable as an attribute instead that receives true or false, you can just rely on the current AttributeExpression (no need to define a new expression).

[vin1antme]

Hello,

Thank you for suggestions.

Have a few questions:

1.How drag event listeners support should properly look like? Should they be separate keywords or parameters of drag_source keyword?

Should it be like this:

label
{
text "label"
drag_source true
on_drag_start {|e| ...}
on_drag_motion {|e| ...}
}

Or like this:

label
{
text "label"
drag_source true :on_drag_start => cmd1, :on_drag_motion => cmd2
}

2.Regarding the sample did not work on Mac just guess that you did not see tooltip showing dragged data and the problem probably related to overrideredirect method not working properly on MacOS.

Unfortunately I have no Mac to test. But following to suggestions from Internet I prepared improved code which may fix the issue.

If you have a chance, could you please replace on_drag_start method from drag_and_drop_expression.rb file with the code from below and see if it change things.

def on_drag_start(event)
     event.widget.configure(:cursor => "hand2")
     event.widget.hint = TkToplevel.new(root)
     event.widget.hint.overrideredirect(1)
     event.widget.hint.overrideredirect(0)
     event.widget.hint.geometry("+#{event.x_root + 4}+#{event.y_root - 2}")
     data = self.text
     TkLabel.new(event.widget.hint) { text data }.pack
end 

3.Also have a question about WidgetProxy#set_attribute method at https://github.com/AndyObtiva/glimmer-dsl-tk/blob/master/lib/glimmer/tk/widget_proxy.rb The last expression in it is send(attribute_setter(attribute), args) . What is the reason of passing arguments args as Array? I ask because expectation was there should be *args or args.first instead.

[AndyObtiva]

1) "How drag event listeners support should properly look like?"

This option is better:

text "label"
drag_source true
on_drag_start {|e| ...}
on_drag_motion {|e| ...}
}

That said, only one of drag_source true or on_drag_start {|e| ...} should be needed, but not both together as that would be redundant. If drag_source true is specified alone, then default behavior is assumed. Otherwise, the user can customize the data transferred through the drag event.

2) "If you have a chance, could you please replace on_drag_start method from drag_and_drop_expression.rb file with the code from below and see if it change things."

I tried to do that on the Mac and drag and drop still did not work unfortunately. I will try your code on a Windows PC when I get a chance over the weekend.

3) "What is the reason of passing arguments args as Array? I ask because expectation was there should be *args or args.first instead."

The reason behind that is because in Ruby, attribute setters that end with the "=" sign usually expect one argument, so I wrapped the args in a single array and passed it to the attribute setter to be treated as one argument.

That said, I just tested in irb passing multiple args to an attribute setter method and it worked when calling it through send, so perhaps I could change that behavior if really needed.

Do you have a strong reason to require changing it?

[vin1antme]

Thank you for clarification! Regarding question 3 - no need for changes, that behavior is okay.

[AndyObtiva]

Regarding question 3, I did some more thinking and decided to splat the args (pass *args) since that works with Ruby 3. It is done on the development branch at the moment until merged to master. Just giving you a heads up.

[vin1antme]

Hello,

I implemented all suggestions. Added support for drag_source true keyword and also for on_drag_start, on_drag_motion, on_drop event listeners as per conversation above. Usage example is in hello_drag_and_drop.rb sample.

[AndyObtiva]

Thank you for following my feedback and suggestions.

I just tested your new sample (hello_drag_and_drop.rb) in all platforms (Mac, Windows, and Linux), and it didn't work in any of them. When I drag the entry, label, or button text, nothing moves or happens.

Could you please provide a brief screen video recording demonstrating your feature (on every platform you tested it on)? Perhaps, I am not using your sample correctly, so I would like to learn directly from you about how to make it work.

[vin1antme]

I tested only on Windows 7 64bit.

Drag happens with right-mouse button pressed.

Here is screenshot with command line where sample is run from my fork.

And here is the video:

https://user-images.githubusercontent.com/14129291/137688330-b0b9aa94-95b1-4d32-8644-4a59115a031a.mp4

[AndyObtiva]

Thank you for the video/screenshot.

"Drag happens with right-mouse button pressed."

Ahhh.. that's a special way to drag, and explains everything! It is useful sometimes, but usually when people think of drag and drop, they are thinking of dragging with the left-mouse-button.

Would you mind changing the sample to use left-mouse-button dragging instead?

We could document right-mouse-button dragging as well in case some software engineers need it, but I think left-mouse-dragging oughta be the default when drag and drop is needed.

One last thing. Could you please test on MacOS and Linux via VirtualBox please?

[vin1antme]

I changed the code and now LEFT-mouse button is used for drag and drop. However during testing found a bug and want to share details.

Steps to reproduce. 1.Drag every "drag source" widget and drop it on Checkbox. 2.Checkbox on_drop handler has event.source.destroy expression, which deletes drag source widget. 3.Every widget will be removed after drop. 4.When "Drag it" button is dropped on the checkbox, the following Tcl error is thrown:

invalid command name ".w00000.w00001.w00009"
invalid command name ".w00000.w00001.w00009"
    while executing
".w00000.w00001.w00009 instate pressed { .w00000.w00001.w00009 state !pressed; .w00000.w00001.w00009 instate !disabled { .w00000.w00001.w00009 invoke }..."
    (command bound to event)

The error is only reproduced when drag source is Button widget and only when dragging happens with LEFT mouse button. The error is not reproduced when drag sources are other widgets and when Button widget is dragged with RIGHT mouse button.

It looks like after Button is destroyed Tcl is trying to handle button-related event handler. I think it is Tcl specific error and it is not caused by Glimmer or drag-and-drop implementation and going to change sample application to avoid that error. Would it be correct resolution?

Regarding testing on other operation systems - unfortunately my PC does not support virtualization, so for a while I can't test Linux and MacOS on virtual machines.

[AndyObtiva]

Thank you.

It works on the Mac!

I saw the error with dropping on the checkbox. You can leave it unsupported for the error case while supporting other cases. I wouldn't worry about it as it is very uncommon to use drag and drop with a checkbox target. Changing the sample application is a good idea.

Cheers.

[AndyObtiva]

This has been tested and confirmed on Mac, Windows, and Linux.

I documented Hello, Drag and Drop! under Samples.

I added you to the list of Contributors on the README as vin1antme. If you have an actual name you'd like to share instead, I'd be happy to revise.