flex-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Harui <aha...@adobe.com>
Subject Re: git commit: [flex-sdk] [refs/heads/develop] - FLEX-16857 Fixed RTE when adding items after setting sort to null
Date Wed, 15 May 2013 04:44:00 GMT
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" <jmclean@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 <jmclean@apache.org>
> Authored: Sat May 11 10:34:47 2013 +1000
> Committer: Justin Mclean <jmclean@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


Mime
View raw message