You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by SYSE | Edvin <es...@syse.no> on 2011/07/23 20:23:15 UTC
Does someone have time to check my patch for PIVOT-276?
Hi guys,
I uploaded a patch for https://issues.apache.org/jira/browse/PIVOT-276
about two weeks ago, that adds a complete solution for a GridView in
Pivot. Did anyone have time to take a look at it? Any comments? :)
Sincerely,
Edvin Syse
Re: Does someone have time to check my patch for PIVOT-276?
Posted by SYSE | Edvin <es...@syse.no>.
OK, thanks Sandro :)
-- Edvin
Den 24.07.2011 22:52, skrev Sandro Martini:
> Hi Edvin,
> I'm just returned, so I'll try your solution in next weeks because
> it's interesting but it's not trivial, so require some time to test it
> ...
>
> Thank you for now.
>
> Bye
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Sandro Martini <sa...@gmail.com>.
Hi Edvin,
I'm just returned, so I'll try your solution in next weeks because
it's interesting but it's not trivial, so require some time to test it
...
Thank you for now.
Bye
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Edvin Syse <es...@syse.no>.
On 08/10/2011 05:02 PM, Chris Bartlett wrote:
> On 10 August 2011 21:26, Chris Bartlett<cb...@gmail.com> wrote:
>> I am not 100% sure of the intended behaviour of Container...
>
> That was supposed to say
> 'I am not 100% sure of the intended behaviour of *this* Container'
>
> But even that would have been wrong, as this is not a Container, but a
> data driven Component.
Hehe.. right :) I'll fix the things you commented on and submit a new
Patch for review, hopefully during this coming weekend.
-- Edvin
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
On 10 August 2011 21:26, Chris Bartlett <cb...@gmail.com> wrote:
> I am not 100% sure of the intended behaviour of Container...
That was supposed to say
'I am not 100% sure of the intended behaviour of *this* Container'
But even that would have been wrong, as this is not a Container, but a
data driven Component.
SV: Does someone have time to check my patch for PIVOT-276?
Posted by Edvin Syse <es...@syse.no>.
>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.
I didn't try, but I looked at the code, and I'm sure it will still work :))
-- Edvin
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
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 <es...@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:cbartlett.x@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 <cb...@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 <es...@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
>>>
>>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
No problem. If you are willing and able to do the hard work, then
someone should be willing to help you out, especially when you turn in
new patches so quickly.
Yes, with those 3 things fixed it should certainly look and feel
right, so now we just have to concentrate on some testing.
I'll try to look at the new version tomorrow.
Chris
On 13 August 2011 04:26, Edvin Syse <es...@syse.no> wrote:
> Thanks again, Chris. I have supplied a new patch that fixes issues 2-4 below.
>
> I'm very grateful that you take the time to test this, it's coming along nicely I think :)
>
> -- Edvin
>
>
>> -----Opprinnelig melding-----
>> Fra: Chris Bartlett [mailto:cbartlett.x@gmail.com]
>> Sendt: 12. august 2011 21:15
>> Til: dev@pivot.apache.org
>> Emne: Re: Does someone have time to check my patch for PIVOT-276?
>>
>> 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
>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
The new patch looks good in ComponentExplorer.
I'll take a look at it more closely as soon as I find the time.
On 13 August 2011 04:26, Edvin Syse <es...@syse.no> wrote:
> Thanks again, Chris. I have supplied a new patch that fixes issues 2-4 below.
>
> I'm very grateful that you take the time to test this, it's coming along nicely I think :)
>
> -- Edvin
>
>
>> -----Opprinnelig melding-----
>> Fra: Chris Bartlett [mailto:cbartlett.x@gmail.com]
>> Sendt: 12. august 2011 21:15
>> Til: dev@pivot.apache.org
>> Emne: Re: Does someone have time to check my patch for PIVOT-276?
>>
>> 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
>
>
SV: Does someone have time to check my patch for PIVOT-276?
Posted by Edvin Syse <es...@syse.no>.
Thanks again, Chris. I have supplied a new patch that fixes issues 2-4 below.
I'm very grateful that you take the time to test this, it's coming along nicely I think :)
-- Edvin
> -----Opprinnelig melding-----
> Fra: Chris Bartlett [mailto:cbartlett.x@gmail.com]
> Sendt: 12. august 2011 21:15
> Til: dev@pivot.apache.org
> Emne: Re: Does someone have time to check my patch for PIVOT-276?
>
> 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
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
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 <es...@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:cbartlett.x@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 <cb...@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 <es...@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
>>>
>>
>
SV: Does someone have time to check my patch for PIVOT-276?
Posted by Edvin Syse <es...@syse.no>.
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:cbartlett.x@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 <cb...@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 <es...@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
>>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Sandro Martini <sa...@gmail.com>.
Hi, I agree with Chris, we can see later in detail some key handling, maybe
configurable even with some style (for example the ability at the end of a
row to stay in the same cell, or to go to the first element of the next row
if any, or to return to the first cell in the same row, etc).
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.
Bye
Il giorno 10/ago/2011 20:43, "Chris Bartlett" <cb...@gmail.com> ha
scritto:
> On 10 August 2011 21:26, Chris Bartlett <cb...@gmail.com> wrote:
>> 1) Up & down arrow keys change the selection, but left & right don't
>> do anything yet
>
> Now that I have played with this a little bit, I think I would expect
> the arrow keys to move the selection in the direction that they
> represent.
>
> So, if there are 4 items in the GridView, and it is currently being
> laid out as a 2x2 grid, pressing DOWN when item[0] is selected would
> move the selection to item[2]. ie, the selection would move from the
> first column of the *first* row to the first column of the the
> *second* row.
>
> Although I use keyboard shortcuts heavily, I would suggest
> concentrating on the layout first and then getting to the keyboard
> handling when the rest of the functionality is complete.
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
On 10 August 2011 21:26, Chris Bartlett <cb...@gmail.com> wrote:
> 1) Up & down arrow keys change the selection, but left & right don't
> do anything yet
Now that I have played with this a little bit, I think I would expect
the arrow keys to move the selection in the direction that they
represent.
So, if there are 4 items in the GridView, and it is currently being
laid out as a 2x2 grid, pressing DOWN when item[0] is selected would
move the selection to item[2]. ie, the selection would move from the
first column of the *first* row to the first column of the the
*second* row.
Although I use keyboard shortcuts heavily, I would suggest
concentrating on the layout first and then getting to the keyboard
handling when the rest of the functionality is complete.
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
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 <cb...@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 <es...@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
>>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
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 <es...@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
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by SYSE | Edvin <es...@syse.no>.
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
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
This seems to have fixed it
Replace
<old>
-import org.apache.pivot.wtk.
;
</old>
with
<new>
-import org.apache.pivot.wtk.ListView;
</new>
On 10 August 2011 19:31, Chris Bartlett <cb...@gmail.com> wrote:
> Should have added that I was using TortoiseSVN to apply the patch, so
> it might be an issue with that - checking now.
>
> On 10 August 2011 19:27, Chris Bartlett <cb...@gmail.com> wrote:
>> Edvin,
>>
>> I've been very busy recently, but just tried to apply your patch to
>> take a look and got an error about a bad line on line 1606
>> It loooks like part of the imports removal has been cut out from
>> wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java
>>
>> Any chance you can put an updated patch onto the JIRA ticket?
>> I will add a comment about the patch error to the JIRA ticket now.
>>
>> Chris
>>
>> On 24 July 2011 01:23, SYSE | Edvin <es...@syse.no> wrote:
>>> Hi guys,
>>>
>>> I uploaded a patch for https://issues.apache.org/jira/browse/PIVOT-276 about
>>> two weeks ago, that adds a complete solution for a GridView in Pivot. Did
>>> anyone have time to take a look at it? Any comments? :)
>>>
>>> Sincerely,
>>> Edvin Syse
>>>
>>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
Should have added that I was using TortoiseSVN to apply the patch, so
it might be an issue with that - checking now.
On 10 August 2011 19:27, Chris Bartlett <cb...@gmail.com> wrote:
> Edvin,
>
> I've been very busy recently, but just tried to apply your patch to
> take a look and got an error about a bad line on line 1606
> It loooks like part of the imports removal has been cut out from
> wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java
>
> Any chance you can put an updated patch onto the JIRA ticket?
> I will add a comment about the patch error to the JIRA ticket now.
>
> Chris
>
> On 24 July 2011 01:23, SYSE | Edvin <es...@syse.no> wrote:
>> Hi guys,
>>
>> I uploaded a patch for https://issues.apache.org/jira/browse/PIVOT-276 about
>> two weeks ago, that adds a complete solution for a GridView in Pivot. Did
>> anyone have time to take a look at it? Any comments? :)
>>
>> Sincerely,
>> Edvin Syse
>>
>
Re: Does someone have time to check my patch for PIVOT-276?
Posted by Chris Bartlett <cb...@gmail.com>.
Edvin,
I've been very busy recently, but just tried to apply your patch to
take a look and got an error about a bad line on line 1606
It loooks like part of the imports removal has been cut out from
wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java
Any chance you can put an updated patch onto the JIRA ticket?
I will add a comment about the patch error to the JIRA ticket now.
Chris
On 24 July 2011 01:23, SYSE | Edvin <es...@syse.no> wrote:
> Hi guys,
>
> I uploaded a patch for https://issues.apache.org/jira/browse/PIVOT-276 about
> two weeks ago, that adds a complete solution for a GridView in Pivot. Did
> anyone have time to take a look at it? Any comments? :)
>
> Sincerely,
> Edvin Syse
>