You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flex.apache.org by jm...@apache.org on 2013/05/11 02:36:48 UTC
git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE
when adding items after setting sort to null
Updated Branches:
refs/heads/develop 9f21583b8 -> 3b98c1d09
FLEX-16857 Fixed RTE when adding items after setting sort to null
Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/3b98c1d0
Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/3b98c1d0
Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/3b98c1d0
Branch: refs/heads/develop
Commit: 3b98c1d096742c4f35b51f28ead2e6827677960a
Parents: 9f21583
Author: Justin Mclean <jm...@apache.org>
Authored: Sat May 11 10:34:47 2013 +1000
Committer: Justin Mclean <jm...@apache.org>
Committed: Sat May 11 10:34:47 2013 +1000
----------------------------------------------------------------------
.../src/mx/collections/ListCollectionView.as | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/3b98c1d0/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
index afce033..dcffed1 100644
--- a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
+++ b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
@@ -378,6 +378,10 @@ public class ListCollectionView extends Proxy
public function set sort(s:ISort):void
{
_sort = s;
+
+ if (s == null)
+ localIndex = null;
+
dispatchEvent(new Event("sortChanged"));
}
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> so if you want I can dig on this after I check on the bound-validators issue.
Leave it with me and I'll get to it tomorrow.
Thanks,
Justin
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
On 5/14/13 11:12 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
> Hi,
>
>> Wow, so there was no way to unsort after sorting in 4.9.1? Or is it the
>> addItem that messes things up?
> Not 100% sure not looked at exact details.
>
> I do know there other issues re adding or removing when sorted or filtered
> when there are duplicates.
>
> Probably needs to be more tests as that test combinations of sorting and
> filtering. I think the "fix" I added may not fix the RTE if you have a filter
> added.
Ugh, yup, there could be a problem there. I'd want to know the root cause
of why unsorting isn't working in case there's something even more
fundamental broken. I'm down to my last mustella failure other than this DG
sort-hack thing, so if you want I can dig on this after I check on the
bound-validators issue.
>
> Justin
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Wow, so there was no way to unsort after sorting in 4.9.1? Or is it the
> addItem that messes things up?
Not 100% sure not looked at exact details.
I do know there other issues re adding or removing when sorted or filtered when there are duplicates.
Probably needs to be more tests as that test combinations of sorting and filtering. I think the "fix" I added may not fix the RTE if you have a filter added.
Justin
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
On 5/14/13 10:43 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>
> And with this code. Items are still sorted after refresh when they shouldn't
> be.
>
> a.sort = null;
> a.refresh();
> a.addItem({...});
>
> The fix I applied for FLEX-16857 "fixes" both of these issues. ie no RTE in
> case one and item are unsorted in case two.
Wow, so there was no way to unsort after sorting in 4.9.1? Or is it the
addItem that messes things up?
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Posted by Justin Mclean <ju...@classsoftware.com>.
HI,
Just so it's clear.
In Apache Flex 4.9.1 - this code gives an RTE.
a.sort = null;
a.addItem({...});
And with this code. Items are still sorted after refresh when they shouldn't be.
a.sort = null;
a.refresh();
a.addItem({...});
The fix I applied for FLEX-16857 "fixes" both of these issues. ie no RTE in case one and item are unsorted in case two.
Thanks,
Justin
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
On 5/14/13 10:33 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
> Hi,
>
>>> I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
>>> refresh after setting sort = null and before adding items. I haven't tried
>>> it, but I suspect that would avoid the error.
>> That is broken as well the objects are still sorted rather than reverting to
>> their original order.
>
> First line of internalRefresh called by refresh causes the "new" issue:
>
> private function internalRefresh(dispatch:Boolean):Boolean
> {
> if (sort || filterFunction != null)
>
I'm not following you. Isn't sort null and filterFunction null? That
should cause it to jump to the else and set localIndex = null.
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
>> I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
>> refresh after setting sort = null and before adding items. I haven't tried
>> it, but I suspect that would avoid the error.
> That is broken as well the objects are still sorted rather than reverting to their original order.
First line of internalRefresh called by refresh causes the "new" issue:
private function internalRefresh(dispatch:Boolean):Boolean
{
if (sort || filterFunction != null)
Justin
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
On 5/14/13 10:29 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
> Hi,
>
>> I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
>> refresh after setting sort = null and before adding items. I haven't tried
>> it, but I suspect that would avoid the error.
> That is broken as well the objects are still sorted rather than reverting to
> their original order.
That's odd. I would certainly expect more bugs filed if you couldn't undo a
sort.
>
>> When theGridItemEditor goes to save the new data, the editor saves any
>> current sort,
>> sets sort = null, then saves the new, then restores the sort and does not
>> call refresh().
> Ouch. Why does it need to do that it should be able to save the new data
> directly.
IIRC, if you have an MX DG sorted by first name and you change Alex to Zlex,
depending on what you do next, the DG scrolls to the bottom to show its new
position, or when you click on the row below the edited row disappears and
the row you clicked on jumps up. I think they are trying to avoid that in
Spark DG.
>
>> 1) verify that refresh() avoids the RTE in FLEX-16857, revert this change
>> and maybe update the doc
> There no RTE but the list is not left in the correct state. I assume this may
> cause further issues down the track, probably need to fix this as well.
>
>> 2) change the behavior of Spark DG when you edit an item in the sort column.
> I'd for this option but no idea how easy/hard it it right this second.
I would just take the sort-saving logic out. I'll try it and see if other
tests then start to fail.
>
> Thanks,
> Justin
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
> refresh after setting sort = null and before adding items. I haven't tried
> it, but I suspect that would avoid the error.
That is broken as well the objects are still sorted rather than reverting to their original order.
> On the other hand, the Spark DG is doing something questionable.
Only the Spark one? :-)
> When theGridItemEditor goes to save the new data, the editor saves any current sort,
> sets sort = null, then saves the new, then restores the sort and does not
> call refresh().
Ouch. Why does it need to do that it should be able to save the new data directly.
> 1) verify that refresh() avoids the RTE in FLEX-16857, revert this change
> and maybe update the doc
There no RTE but the list is not left in the correct state. I assume this may cause further issues down the track, probably need to fix this as well.
> 2) change the behavior of Spark DG when you edit an item in the sort column.
I'd for this option but no idea how easy/hard it it right this second.
Thanks,
Justin
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
Justin,
Thanks for dealing with the DateValidator failure. I have another one for
you.
This change is causing a failure in the mustella test:
tests/gumbo/components/DataGrid/Properties/DataGrid_Properties_sortable
column_withEditor_test002. Did this test pass for you?
I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
refresh after setting sort = null and before adding items. I haven't tried
it, but I suspect that would avoid the error. The documentation is pretty
clear that calling refresh() is required, although I suppose we could update
the doc to further specify that this is true even when setting sort = null.
On the other hand, the Spark DG is doing something questionable. When the
GridItemEditor goes to save the new data, the editor saves any current sort,
sets sort = null, then saves the new, then restores the sort and does not
call refresh(). As far as I can tell, this is an attempt to retain the
position of the edited item in the Grid. I guess they didn't want the Grid
to jump to a new position or have the edited item disappear off-screen if
the new data caused the sort to move the item in the collection. Because
this change sets localIndex = null right when sort = null, the localIndex is
tossed and the new "order" is unsorted order and the test fails.
So, I think there are two options:
1) verify that refresh() avoids the RTE in FLEX-16857, revert this change
and maybe update the doc
2) change the behavior of Spark DG when you edit an item in the sort column.
Thoughts? I'm pretty sure MX DG jumps around when you edit an item in the
sort column, but this would be a change in behavior for Spark DG users.
I'd probably choose #1 as maybe there are other folks relying on localIndex
staying around until you call refresh(), but if you feel strongly that
ListCollectionView should be more tolerant of setting sort=null and not
calling refresh() and it is ok to change behavior on Spark DG users, I'd be
ok with that (and of course, would pass an complaints on to you :-)).
-Alex
On 5/10/13 5:36 PM, "jmclean@apache.org" <jm...@apache.org> wrote:
> Updated Branches:
> refs/heads/develop 9f21583b8 -> 3b98c1d09
>
>
> FLEX-16857 Fixed RTE when adding items after setting sort to null
>
>
> Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
> Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/3b98c1d0
> Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/3b98c1d0
> Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/3b98c1d0
>
> Branch: refs/heads/develop
> Commit: 3b98c1d096742c4f35b51f28ead2e6827677960a
> Parents: 9f21583
> Author: Justin Mclean <jm...@apache.org>
> Authored: Sat May 11 10:34:47 2013 +1000
> Committer: Justin Mclean <jm...@apache.org>
> Committed: Sat May 11 10:34:47 2013 +1000
>
> ----------------------------------------------------------------------
> .../src/mx/collections/ListCollectionView.as | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/3b98c1d0/frameworks/proje
> cts/framework/src/mx/collections/ListCollectionView.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> index afce033..dcffed1 100644
> --- a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> +++ b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> @@ -378,6 +378,10 @@ public class ListCollectionView extends Proxy
> public function set sort(s:ISort):void
> {
> _sort = s;
> +
> + if (s == null)
> + localIndex = null;
> +
> dispatchEvent(new Event("sortChanged"));
> }
>
>
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui
Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed
RTE when adding items after setting sort to null
Posted by Alex Harui <ah...@adobe.com>.
Justin,
Thanks for dealing with the DateValidator failure. I have another one for
you.
This change is causing a failure in the mustella test:
tests/gumbo/components/DataGrid/Properties/DataGrid_Properties_sortable
column_withEditor_test002. Did this test pass for you?
I looked at FLEX-16857, and IMO, it is not a bug. The code is not calling
refresh after setting sort = null and before adding items. I haven't tried
it, but I suspect that would avoid the error. The documentation is pretty
clear that calling refresh() is required, although I suppose we could update
the doc to further specify that this is true even when setting sort = null.
On the other hand, the Spark DG is doing something questionable. When the
GridItemEditor goes to save the new data, the editor saves any current sort,
sets sort = null, then saves the new, then restores the sort and does not
call refresh(). As far as I can tell, this is an attempt to retain the
position of the edited item in the Grid. I guess they didn't want the Grid
to jump to a new position or have the edited item disappear off-screen if
the new data caused the sort to move the item in the collection. Because
this change sets localIndex = null right when sort = null, the localIndex is
tossed and the new "order" is unsorted order and the test fails.
So, I think there are two options:
1) verify that refresh() avoids the RTE in FLEX-16857, revert this change
and maybe update the doc
2) change the behavior of Spark DG when you edit an item in the sort column.
Thoughts? I'm pretty sure MX DG jumps around when you edit an item in the
sort column, but this would be a change in behavior for Spark DG users.
I'd probably choose #1 as maybe there are other folks relying on localIndex
staying around until you call refresh(), but if you feel strongly that
ListCollectionView should be more tolerant of setting sort=null and not
calling refresh() and it is ok to change behavior on Spark DG users, I'd be
ok with that (and of course, would pass an complaints on to you :-)).
-Alex
On 5/10/13 5:36 PM, "jmclean@apache.org" <jm...@apache.org> wrote:
> Updated Branches:
> refs/heads/develop 9f21583b8 -> 3b98c1d09
>
>
> FLEX-16857 Fixed RTE when adding items after setting sort to null
>
>
> Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
> Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/3b98c1d0
> Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/3b98c1d0
> Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/3b98c1d0
>
> Branch: refs/heads/develop
> Commit: 3b98c1d096742c4f35b51f28ead2e6827677960a
> Parents: 9f21583
> Author: Justin Mclean <jm...@apache.org>
> Authored: Sat May 11 10:34:47 2013 +1000
> Committer: Justin Mclean <jm...@apache.org>
> Committed: Sat May 11 10:34:47 2013 +1000
>
> ----------------------------------------------------------------------
> .../src/mx/collections/ListCollectionView.as | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/3b98c1d0/frameworks/proje
> cts/framework/src/mx/collections/ListCollectionView.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> index afce033..dcffed1 100644
> --- a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> +++ b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as
> @@ -378,6 +378,10 @@ public class ListCollectionView extends Proxy
> public function set sort(s:ISort):void
> {
> _sort = s;
> +
> + if (s == null)
> + localIndex = null;
> +
> dispatchEvent(new Event("sortChanged"));
> }
>
>
--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui