That was quick! I will have another look as soon as I can find the time. I am currently trying to finish off some Pivot event related unit test code, so this new Component would be a perfect candidate to get a full set of event/ListenerList tests.
Did you check to see if it still works with the ComponentExplorer patch I added? I imagine it will do as my patch doesn't do much more that create an instance and give it some data. Chris On 12 August 2011 23:33, Edvin Syse <e...@syse.no> wrote: > Hi Chris, > > thanks for taking the time to look at this. > > I just uploaded a new patch that fixes: > > - Import error in last patch > - Arrow keys for navigation (up, down, left, right) > - Removed 'alternateItemBackgroundColor' - makes no sense > - Fixed padding related errors for hover, selection and layout > - Added context menu to remove selected item, or add more items at random > locations, to show update functionality > > On on 10.08.2011 21:36 Sandro wrote: >> And, important, will it be possible to add/remove rows and columns ? Using a >> single array for all cells could make things a little complex here ... just >> as quick ideas. > > You can add/remove items on the fly. What do you envision when you say > add/remove rows and columns? The number of rows and columns will depend on > the size available for the GridView, and that will change when you resize the > container. What did you have in mind? > > -- Edvin > > -----Opprinnelig melding----- > Fra: Chris Bartlett [mailto:cbartlet...@gmail.com] > Sendt: 10. august 2011 16:27 > Til: Pivot Dev > Emne: Re: Does someone have time to check my patch for PIVOT-276? > > Edvin, > > Firstly, it looks good, especially as you are so new to Pivot. Good work. > Secondly, my initial thoughts/comments below just come from what I see. I am > not 100% sure of the intended behaviour of Container, so feel free to ignore > any that don't make sense. > > 1) Up & down arrow keys change the selection, but left & right don't do > anything yet > > 2) The skin has an 'alternateItemBackgroundColor' style but when I set it, it > seems to apply to all items, or some rows depending on how the window is > sized. > > 3) The layout looks like it takes the 'horizontalSpacing' & 'verticalSpacing' > values into account, but not the padding values. > If I set the left side padding to be a large number, the grid is pushed > right, and some items can be rendered off screen and are not moved so that > they remain visible. If I resize the window, then items are moved, but some > can still be pushed off to the right and not visible. > > 4) The mouse over highlighting & selection using the mouse seem to have > problems with the padding values too. Set the left padding to a large value > to make the issue more obvious > > I saw some odd rendering issues, but they might just be because of the > layout/padding stuff, so it is probably best to look into at that first, and > then I can re-test and record some screencasts if needed. > > > I haven't looked at code, written an unit tests or tried things like binding > yet. The above comments are from playing around with the GridView component > and tweaking skin values. I will try to knock up a quick BXML version and a > patch so that it can be used with ComponentExplorer which should help when > testing. > > Chris > > On 10 August 2011 19:55, Chris Bartlett <cbartlet...@gmail.com> wrote: >> Yeah, I can apply the patch and just ran the test app. It failed >> first time (probably an iTunes issue) so just having a quick look at >> the functionality now. I won't have time for a complete code review >> or anything right now, but didn't want you to think that patches are >> just forgotten once they are supplied. :) >> >> It would be good to add an updated, working, patch anyway. >> Also, try to ensure that patches don't 'compress' the imports by using >> wildcards. (import org.apache.pivot.wtk.*;) >> >> This is probably documented somewhere, but I can't think where. >> It might just be part of the Pivot Eclipse code formatting rules which >> won't help non-Eclipse users much. >> >> On 10 August 2011 19:44, SYSE | Edvin <e...@syse.no> wrote: >>> Den 10.08.2011 14:40, skrev Chris Bartlett: >>>> >>>> This seems to have fixed it >>>> >>>> Replace >>>> <old> >>>> -import org.apache.pivot.wtk. >>>> ; >>>> </old> >>>> >>>> with >>>> <new> >>>> -import org.apache.pivot.wtk.ListView; </new> >>> >>> Ah.. I'm very sorry :) Did it compile now? Please tell me how it goes! >>> >>> -- Edvin >>> >> >
