Edvin, OK, again just the results of a quick look (and the new patch does still work fine with ComponentExplorer) I'll try to dig a little deeper next time and go further than just tweaking values in ComponentExplorer.
1) Layout looks good now regardless of padding & spacing values 2) Changes to the 'horizontalSpacing' and 'verticalSpacing' styles do not invalidate the component. see org.apache.pivot.wtk.skin.terra.TerraGridViewSkin.setPadding(Insets) or similar for other styles that do invalidate when changed You can see this when you change their values in ComponentExplorer 3) Pressing the DOWN arrow can lead to a java.lang.IndexOutOfBoundsException when there is no item below the selection. See the attached screenshot. 4) Expanding the selection of items in MULTI select mode is currently a bit weird (due to the changes in the keyboard/arrow key processing), but I expect you haven't got to that yet. It is probably best to see how other UI platforms handle this for components similar to GridView The 'alternateItemBackgroundColor' style might be useful, but it can always be added later if/when someone requests it. 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 >>> >> >
