shoes / shoes3

a tiny graphical app kit for ruby
http://walkabout.mvmanila.com
Other
181 stars 19 forks source link

list_box not appending items #45

Open ProvGANG opened 9 years ago

ProvGANG commented 9 years ago

When you try to append items to a list_box like it is a hash, nothing happens. Here is the code sample of this issue.

list

IanTrudel commented 9 years ago

Thank you for your contribution, Luka. You can write code with syntax highlighting on this issue tracker using _```ruby before and **```**_ after your code.

It is necessary to update the list using items method as shown below. The ListBox is actually updated as it should but the element on screen is not updated. It is indeed an issue.

Shoes.app do
   @l = list_box :items => ["first", "second"], :choose => "first"

   @e = edit_line

   button "ok" do
      @l.items << @e.text
   end
end
IanTrudel commented 9 years ago

image

image

IanTrudel commented 9 years ago

TEMPORARY WORKAROUND This is a temporary workaround until it gets fixed in Shoes C-level implementation. This workaround basically creates another ListBox on top of the existing one but with the new items and then removes the old ListBox. Works on Windows and MacOS X.

Shoes.app do
   @f = flow :width => 250 do
      @l = list_box :items => ["first", "second"], :choose => "first"
   end

   @e = edit_line

   button "ok" do
      unless @e.text.empty?
         n = @f.list_box :items => @l.items + [@e.text], :choose => @l.text
         temp = @l
         @l = n
         temp.remove
      end
   end
end
ProvGANG commented 9 years ago

Alright thank you, I wasn't sure that @l.items should be included. Thanks for your solution as I haven't used list_box since I've found an issue long time before. It was reported to a bug report of shoes team, but I don't remember was it for shoes4.

2015-01-27 0:05 GMT+01:00 BackOrder notifications@github.com:

TEMPORARY WORKAROUND This is a temporary workaround until it gets fixed in Shoes C-level implementation. This workaround basically creates another ListBox on top of the existing one but with the new items and then removes the old ListBox. Works on Windows and MacOS X.

Shoes.app do s = flow :width => 250 do @l = list_box :items => ["first", "second"], :choose => "first" end

@e = edit_line

button "ok" do @l.items << @e.text n = s.list_box :items => @l.items, :choose => @l.text temp = @l @l = n temp.remove endend

— Reply to this email directly or view it on GitHub https://github.com/Shoes3/shoes3/issues/45#issuecomment-71557778.

Cheers!

IanTrudel commented 9 years ago

No problem, Luka. Keep up filing bug reports. This is helpful in improving Shoes.

passenger94 commented 9 years ago

hello, Thanks a lot All for your work !!!

The test code isn't working for me too on Ubuntu 14.04 + latest shoes

for a workaround i found this, which works for me

Shoes.app do
   @l = list_box :items => ["first", "second"], :choose => "first"

   @e = edit_line

   button "ok" do
      @l.items << @e.text
      @l.items = @l.items
   end
end
IanTrudel commented 9 years ago

Great find @passenger94 ! It is even better than my workaround. Confirmed working on Windows and MacOS X.

IanTrudel commented 9 years ago

You can change both lines for one @l.items += [@e.text] based on your workaround.

passenger94 commented 9 years ago

oh yes ! but of course :-D

IanTrudel commented 9 years ago

@ccoupe

Shoes defines ListBox#items= to create a list of items in C. This works as expected. Since ListBox#items returns an array, it is possible to perform array operations on it such as <<. What the ListBox class does not do is monitoring changes on its own internal array representation of items.

This issue is likely to happen to any other similar methods in Shoes.

ccoupe commented 9 years ago

I'm going to give an educated guess about whats really going on. It's a ruby array and << is handled by ruby, not shoes. After the append Ruby, doesn't tell shoes - nor would you expect it to for every Ruby array << . The += works because after Ruby appends to the array, it sets (= part) which does look for the object to set which is a Shoes list.

I Think the Shoes 4 folks just went through this problem and concluded it's a nasty problem. Along with compatibility issues of course.

IanTrudel commented 9 years ago

Yes, this is essentially what I am saying. MVC pattern would use an observer to notify changes. It might be possible to subclass Array and override << methods to become ItemsArray. Then it will be possible to know whenever changes happen. Why did you remove Windows label here? It does affect all platforms.

ccoupe commented 9 years ago

I think I'll note the behavior and solution in the manual (Shoes only looks like Ruby). I'm not in a hurry to subclass all of Array with C methods where this problem could occur like list.items[2]= 'new'

Patches welcome of course.

IanTrudel commented 9 years ago

Good idea to put such things in the manual. For this issue, we might have to think before patching in such way that we can monitor internal changes on anything we need to. I suspect other classes suffer from this in many more ways than just what we are seeing with ListBox.

ccoupe commented 9 years ago

I think listbox is the only widget that returns an array where folks would assume Ruby array options would work on Shoes internals. I don't think we should close the issue since many others will stumble on the problem. I remember some head scratching of my own on this issue a few years ago.

IanTrudel commented 9 years ago

Many other Ruby projects will subclass array and override methods. It would be possible to write something with an observe. Provided that we would eventually do that, shall we keep this issue open?

https://www.ruby-forum.com/topic/114260

ccoupe commented 9 years ago

If you feel strongly.

IanTrudel commented 9 years ago

It seems the right thing to do for a smooth usage and it can be implemented at Ruby level rather than C level. Worthy to explore in my opinion. Where would you load such feature? Loaded in https://github.com/Shoes3/shoes3/blob/master/lib/shoes.rb and then the actual code in https://github.com/Shoes3/shoes3/tree/master/lib/shoes ?

ccoupe commented 9 years ago

ruby coders may think list_box it is an Array but It is not an Array. You can refine/subclass/monkey patch Array to your hearts content and it means nothing to the opaque swt/gtk/cocoa/msw widgets internal structure without a way to tell the native widget to redraw/layout/recreate with new contents.

It might not be hard to expose a new set of list_box.add/delete/repaint methods from the gtk.c code (cocoa?) but there is still a hell of a lot of Array methods to override to insure that those methods get called in the correct manner (e.g. after collect or for each internal interation in the block?). And it needs to be thread safe and gc safe.

Explore away. Design the gtk api to match.

IanTrudel commented 9 years ago

Thanks for the feedback. According to the link, it should be possible to simply using method_missing in the subclass and a notifier using Ruby standard observer pattern. You are however correct about the repaint. The observer pattern would have to notify gtk/cocoa code to repaint.

NOTES

REFERENCES https://github.com/Shoes3/shoes3/blob/master/shoes/ruby.c#L4723 (items= method) https://github.com/Shoes3/shoes3/blob/master/shoes/ruby.c#L3270 (shoes_list_box_items_set) https://github.com/Shoes3/shoes3/blob/master/shoes/native/gtk.c#L1291 (shoes_native_list_box_update) https://github.com/Shoes3/shoes3/blob/master/shoes/native/cocoa.m#L1305 (shoes_native_list_box_update)

ccoupe commented 9 years ago
 Shoes.app do
   para "Choose a fruit:"
   @lb = list_box :items => ["Grapes", "Pears", "Apricots"]
   @lb.items << 'kumquats'
   @lb.items[2] = 'Kiwi'
   @lb.items.pop
   @lb.items.collect! { |x| "the correct name is:  " + Fruit.lookup(x)} 
   button "Report Flicker..."
 end
IanTrudel commented 9 years ago

Exploring using a custom Shoes::Widget and an observer pattern.

require("observer")

class Element
  include Observable

  def initialize(array)
    @array = array
  end

  def method_missing(meth, *args, &block)
    @array.send(meth, *args, &block)
    changed
    notify_observers(@array, meth, *args, &block)
  end
end

class CustomListBox < Shoes::Widget
  attr_accessor :items

  def initialize(options = {}, &block)
    @lb = list_box options
    @items = Element.new(@lb.items)
    @items.add_observer(self)
  end

  def method_missing(meth, *args, &block)
    if @lb.respond_to? meth
      @lb.send(meth, *args, &block)
    else
      super
    end
  end

  def update(array, *call)
    @lb.items = array
  end
end

Shoes.app {
  @l = custom_list_box :items => ["first", "second"], :choose => "first"
  @e = edit_line

  button "ok" do
    @l.items << @e.text
  end

  button "collect" do
    @l.items.collect! { |n| n.reverse }
  end
}
ccoupe commented 9 years ago

That would make an excellent samples/expert-custom-list-box.rb or some such name. If it's robust for all Array methods, and all listbox methods ( not in the class chain) and Common methods then it could live in lib/shoes and be required from lib/shoes.rb. and documented in the Elements or Better Widgets section of the Manual.

If you envision a lot of these 'custom' widgets then we probably should think about what to call them and pick good Classnames . Or they could be distributed separately from Shoe as something users can put in their own projects .

IanTrudel commented 9 years ago

This is a great suggestion @ccoupe ! I committed a slightly modified version to the samples directory.

As for the inclusion to the core feature, this is to something to think about first. You previously mentioned that ListBox#items, ListBox#items= are the only array used. It would mean that we could implement the observer within Shoes and keep the sample as a sample.

Might be a good thing for the manual. Or at least how to create a custom Widget.

To be noted that custom widgets seem to behave singly differently. For example, I could not wrap the CustomListBox properly into a flow and the rendering didn't look as the one using ListBox, which included the ListBox, EditBox and Button all on the same horizontal line.

An interesting exploration so far.