You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by OmPrakash Muppirala <bi...@gmail.com> on 2013/11/13 03:09:47 UTC

Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

This is my first real contribution to the FlexJS project.  Alex/Peter can
you please review this and let me know if I am on the right track?
Once you approve, I will start looking at the JS version of the same
logic.

Thanks,
Om


On Tue, Nov 12, 2013 at 6:07 PM, <bi...@apache.org> wrote:

> Updated Branches:
>   refs/heads/develop 22f891823 -> 67c11796e
>
>
> Add functionality to AS version of DataGrid to highlight entire row when
> rolling over any column
> JS version to follow after discussion on dev@f.a.o
>
>
> Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
> Commit: http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/67c11796
> Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/67c11796
> Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/67c11796
>
> Branch: refs/heads/develop
> Commit: 67c11796e271bf7811e8afb12f556aa2386d2960
> Parents: 22f8918
> Author: Om <bi...@gmail.com>
> Authored: Tue Nov 12 18:05:48 2013 -0800
> Committer: Om <bi...@gmail.com>
> Committed: Tue Nov 12 18:06:26 2013 -0800
>
> ----------------------------------------------------------------------
>  .../src/org/apache/flex/core/IRollOverModel.as  | 28 ++++++++++++++++++++
>  .../org/apache/flex/html/staticControls/List.as | 12 ++++++++-
>  .../html/staticControls/beads/DataGridView.as   | 13 +++++++++
>  .../flex/html/staticControls/beads/ListView.as  | 19 +++++++++++++
>  .../controllers/ItemRendererMouseController.as  |  1 +
>  .../ListSingleSelectionMouseController.as       | 10 ++++++-
>  .../beads/models/ArraySelectionModel.as         | 14 +++++++++-
>  7 files changed, 94 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> ----------------------------------------------------------------------
> diff --git a/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> new file mode 100644
> index 0000000..d38aa41
> --- /dev/null
> +++ b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> @@ -0,0 +1,28 @@
>
> +////////////////////////////////////////////////////////////////////////////////
> +//
> +//  Licensed to the Apache Software Foundation (ASF) under one or more
> +//  contributor license agreements.  See the NOTICE file distributed with
> +//  this work for additional information regarding copyright ownership.
> +//  The ASF licenses this file to You under the Apache License, Version
> 2.0
> +//  (the "License"); you may not use this file except in compliance with
> +//  the License.  You may obtain a copy of the License at
> +//
> +//      http://www.apache.org/licenses/LICENSE-2.0
> +//
> +//  Unless required by applicable law or agreed to in writing, software
> +//  distributed under the License is distributed on an "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> +//  See the License for the specific language governing permissions and
> +//  limitations under the License.
> +//
>
> +////////////////////////////////////////////////////////////////////////////////
> +package org.apache.flex.core
> +{
> +       import org.apache.flex.events.IEventDispatcher;
> +
> +       public interface IRollOverModel extends IEventDispatcher,
> IBeadModel
> +       {
> +               function get rollOverIndex():int;
> +               function set rollOverIndex(value:int):void;
> +       }
> +}
> \ No newline at end of file
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> ----------------------------------------------------------------------
> diff --git a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> index 7cb3b6d..da1638e 100644
> --- a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> +++ b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> @@ -18,8 +18,9 @@
>
>  ////////////////////////////////////////////////////////////////////////////////
>  package org.apache.flex.html.staticControls
>  {
> +       import org.apache.flex.core.IRollOverModel;
>         import org.apache.flex.core.ISelectionModel;
> -    import org.apache.flex.core.UIBase;
> +       import org.apache.flex.core.UIBase;
>         import org.apache.flex.core.ValuesManager;
>         import
> org.apache.flex.html.staticControls.beads.IDataProviderItemRendererMapper;
>
> @@ -56,6 +57,15 @@ package org.apache.flex.html.staticControls
>                 {
>                         ISelectionModel(model).selectedIndex = value;
>                 }
> +
> +        public function get rollOverIndex():int
> +               {
> +                       return IRollOverModel(model).rollOverIndex;
> +               }
> +               public function set rollOverIndex(value:int):void
> +               {
> +                       IRollOverModel(model).rollOverIndex = value;
> +               }
>
>                 public function get selectedItem():Object
>                 {
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> index 4088734..f2d3cc2 100644
> ---
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> +++
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> @@ -85,6 +85,7 @@ package org.apache.flex.html.staticControls.beads
>                                 columns.push(column);
>
>
> column.addEventListener('change',columnListChangeHandler);
> +
> column.addEventListener('rollover',columnListRollOverHandler);
>                         }
>
>
> IEventDispatcher(_strand).addEventListener("widthChanged",handleSizeChange);
> @@ -139,5 +140,17 @@ package org.apache.flex.html.staticControls.beads
>
>                         IEventDispatcher(_strand).dispatchEvent(new
> Event('change'));
>                 }
> +
> +               private function
> columnListRollOverHandler(event:Event):void
> +               {
> +                       var list:List = event.target as List;
> +                       for(var i:int=0; i < columns.length; i++) {
> +                               if (list != columns[i]) {
> +                                       columns[i].rollOverIndex =
> list.rollOverIndex;
> +                               }
> +                       }
> +
> +                       IEventDispatcher(_strand).dispatchEvent(new
> Event('rollOver'));
> +               }
>         }
>  }
> \ No newline at end of file
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> index dbe9e36..b8c9ab9 100644
> ---
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> +++
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> @@ -27,6 +27,7 @@ package org.apache.flex.html.staticControls.beads
>         import org.apache.flex.core.IItemRendererParent;
>         import org.apache.flex.core.ILayoutParent;
>         import org.apache.flex.core.IParent;
> +       import org.apache.flex.core.IRollOverModel;
>         import org.apache.flex.core.ISelectionModel;
>         import org.apache.flex.core.IStrand;
>         import org.apache.flex.core.Strand;
> @@ -96,6 +97,7 @@ package org.apache.flex.html.staticControls.beads
>
>              listModel = value.getBeadByType(ISelectionModel) as
> ISelectionModel;
>              listModel.addEventListener("selectedIndexChanged",
> selectionChangeHandler);
> +            listModel.addEventListener("rollOverIndexChanged",
> rollOverIndexChangeHandler);
>
>              _border = new Border();
>              _border.model = new SingleLineBorderModel();
> @@ -128,6 +130,23 @@ package org.apache.flex.html.staticControls.beads
>                         }
>              lastSelectedIndex = listModel.selectedIndex;
>                 }
> +
> +               private var lastRollOverIndex:int = -1;
> +
> +               private function
> rollOverIndexChangeHandler(event:Event):void
> +               {
> +                       if (lastRollOverIndex != -1)
> +                       {
> +                               var ir:IItemRenderer =
> dataGroup.getItemRendererForIndex(lastRollOverIndex) as IItemRenderer;
> +                ir.hovered = false;
> +                       }
> +                       if (IRollOverModel(listModel).rollOverIndex != -1)
> +                       {
> +                   ir =
> dataGroup.getItemRendererForIndex(IRollOverModel(listModel).rollOverIndex);
> +                   ir.hovered = true;
> +                       }
> +                       lastRollOverIndex =
> IRollOverModel(listModel).rollOverIndex;
> +               }
>
>                 private function createScrollBar():ScrollBar
>                 {
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendererMouseController.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendererMouseController.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendererMouseController.as
> index 87d9a58..fbc0b1c 100644
> ---
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendererMouseController.as
> +++
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendererMouseController.as
> @@ -49,6 +49,7 @@ package
> org.apache.flex.html.staticControls.beads.controllers
>                         if (target)
>                         {
>                  target.hovered = true;
> +                               target.dispatchEvent(new
> Event("rollover"));
>                         }
>                 }
>
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleSelectionMouseController.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleSelectionMouseController.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleSelectionMouseController.as
> index dc69476..ea870ab 100644
> ---
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleSelectionMouseController.as
> +++
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleSelectionMouseController.as
> @@ -21,11 +21,12 @@ package
> org.apache.flex.html.staticControls.beads.controllers
>         import org.apache.flex.core.IBeadController;
>         import org.apache.flex.core.IItemRenderer;
>         import org.apache.flex.core.IItemRendererParent;
> +       import org.apache.flex.core.IRollOverModel;
>         import org.apache.flex.core.ISelectionModel;
>         import org.apache.flex.core.IStrand;
> -       import org.apache.flex.html.staticControls.beads.IListView;
>         import org.apache.flex.events.Event;
>         import org.apache.flex.events.IEventDispatcher;
> +       import org.apache.flex.html.staticControls.beads.IListView;
>
>
>         public class ListSingleSelectionMouseController implements
> IBeadController
> @@ -47,6 +48,7 @@ package
> org.apache.flex.html.staticControls.beads.controllers
>                         listView = value.getBeadByType(IListView) as
> IListView;
>                         dataGroup = listView.dataGroup;
>              dataGroup.addEventListener("selected", selectedHandler, true);
> +            dataGroup.addEventListener("rollover", rolloverHandler, true);
>                 }
>
>          private function selectedHandler(event:Event):void
> @@ -54,6 +56,12 @@ package
> org.apache.flex.html.staticControls.beads.controllers
>              listModel.selectedIndex = IItemRenderer(event.target).index;
>              IEventDispatcher(listView.strand).dispatchEvent(new
> Event("change"));
>          }
> +
> +        private function rolloverHandler(event:Event):void
> +        {
> +            IRollOverModel(listModel).rollOverIndex =
> IItemRenderer(event.target).index;
> +            IEventDispatcher(listView.strand).dispatchEvent(new
> Event("rollover"));
> +        }
>
>         }
>  }
> \ No newline at end of file
>
>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionModel.as
> ----------------------------------------------------------------------
> diff --git
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionModel.as
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionModel.as
> index 3ca7f94..d1794be 100644
> ---
> a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionModel.as
> +++
> b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionModel.as
> @@ -18,12 +18,13 @@
>
>  ////////////////////////////////////////////////////////////////////////////////
>  package org.apache.flex.html.staticControls.beads.models
>  {
> +       import org.apache.flex.core.IRollOverModel;
>         import org.apache.flex.core.ISelectionModel;
>         import org.apache.flex.core.IStrand;
>         import org.apache.flex.events.Event;
>         import org.apache.flex.events.EventDispatcher;
>
> -       public class ArraySelectionModel extends EventDispatcher
> implements ISelectionModel
> +       public class ArraySelectionModel extends EventDispatcher
> implements ISelectionModel, IRollOverModel
>         {
>                 public function ArraySelectionModel()
>                 {
> @@ -49,6 +50,7 @@ package org.apache.flex.html.staticControls.beads.models
>                 }
>
>                 private var _selectedIndex:int = -1;
> +               private var _rollOverIndex:int = -1;
>
>                 public function get selectedIndex():int
>                 {
> @@ -61,6 +63,16 @@ package org.apache.flex.html.staticControls.beads.models
>                         dispatchEvent(new Event("selectedIndexChanged"));
>                 }
>
> +               public function get rollOverIndex():int
> +               {
> +                       return _rollOverIndex;
> +               }
> +               public function set rollOverIndex(value:int):void
> +               {
> +                       _rollOverIndex = value;
> +                       dispatchEvent(new Event("rollOverIndexChanged"));
> +               }
> +
>                 private var _selectedItem:Object;
>
>                 public function get selectedItem():Object
>
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.

On 11/14/13 12:37 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>On Thu, Nov 14, 2013 at 12:19 PM, Peter Ent <pe...@adobe.com> wrote:
>
>>
>>
>> On 11/14/13 2:55 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>>
>> >On Thu, Nov 14, 2013 at 11:45 AM, Alex Harui <ah...@adobe.com> wrote:
>> >
>> >>
>> >>
>> >> On 11/14/13 10:57 AM, "OmPrakash Muppirala" <bi...@gmail.com>
>> >>wrote:
>> >>
>> >> >On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com>
>>wrote:
>> >> >
>> >> >>
>> >> >>
>> >> >> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com>
>> >> wrote:
>> >> >>
>> >> >> >I think I get it, but I guess I will need some pseudo code to
>> >> >>understand
>> >> >> >completely.  What exactly does 'hang it on a strand' man in this
>> >> >>context?
>> >> >> >
>> >> >> >Thanks,
>> >> >> >Om
>> >> >> I looked at the commit diffs again.  I missed seeing earlier that
>>you
>> >> >>want
>> >> >> to propagate rolloverindex to the List's API surface in order to
>> >> >> communicate that information to the DataGrid.  I was thinking that
>> >>there
>> >> >> can be some other List that represents the List that is a column
>>in
>> >>the
>> >> >> DG.  In theory it can be built out of most of the same pieces as
>> >>List.
>> >> >>
>> >> >
>> >> >I wanted to do that anyways.  Right now, when there is more than a
>> >>page of
>> >> >data, three scrollbars (for 3 columns) show up.  I wanted to create
>>a
>> >> >pared
>> >> >down version of ListView that takes in an optional ScrollBar.  And
>> >>allow
>> >> >the DataGridView to have a scrollbar.  Any thoughts on that?
>> >> Does the ScrollBar need to be optional for ListView?  IMO, the custom
>> >> ListView wouldn't have a scrollbar at all.
>> >>
>> >
>> >(Option 1:)
>> >I thought the ScrollBar could be a view bead that we can inject into
>> >ListView as needed.
>> >
>> >(Option 2:)
>> >Or do you mean that we create a new DataGridColumnListView with
>>everything
>> >in ListView except for ScrollBar and add RollOver behavior like you
>> >mentioned earlier?
>> >
>> >Option 2 means that we are not DRY.  With option 1, we can use a
>>ListView
>> >in mobile as well where there is no need for a visible scrollbar.
>>
>> I think Option 2 is best or the ListView has its Scrollbar removed and
>>if
>> you want a scrollable list you add that as a bead to the List strand. We
>> could then package a ScrollableList which would probably be the one most
>> people use.
>>
>
>Your phrasing is a bit confusing.  Are you proposing an option 3:
>"ListView
>has its Scrollbar removed and if
>you want a scrollable list you add that as a bead to the List strand. " ?
>
>In that case, I like option 3.  If I understand correct, it is like
>option1
>except you want the scrollbar bead to be added to the List and not
>ListView.

In theory, if you get your beads encapsulated properly, then we'll still
be DRY.  The code that actually does the scrolling is not repeated.  IMO,
it isn't repeating yourself to have similar configurations of beads.

For sure, there should be a List or ListView that has a scrollbar and one
without.  Whether scrolling is added to the List or ListView I don't think
I have a set opinion on.  We might create a ListWithScrollBars top-level
component or maybe the scrolling implementation is specified in CSS so it
can not be installed in mobile configs.  Or maybe there is a ListView and
ListViewWithScrollBars.  The key point is that the ListView code doesn't
have ScrollBar logic baked into it that maybe doesn't run based on some
configuration.  The goal is for that ScrollBar logic to be encapsulated in
a bead.  There's another implementation of a Scrolling UI which is the
bumper bars at the top/bottom of a List or Menu.

I think I get where you're coming from.  Build the pieces and if we need
to make separate classes that compose a few together to reduce the amount
of typing, great, if not, even better.

-Alex


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Thu, Nov 14, 2013 at 12:19 PM, Peter Ent <pe...@adobe.com> wrote:

>
>
> On 11/14/13 2:55 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
> >On Thu, Nov 14, 2013 at 11:45 AM, Alex Harui <ah...@adobe.com> wrote:
> >
> >>
> >>
> >> On 11/14/13 10:57 AM, "OmPrakash Muppirala" <bi...@gmail.com>
> >>wrote:
> >>
> >> >On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com> wrote:
> >> >
> >> >>
> >> >>
> >> >> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com>
> >> wrote:
> >> >>
> >> >> >I think I get it, but I guess I will need some pseudo code to
> >> >>understand
> >> >> >completely.  What exactly does 'hang it on a strand' man in this
> >> >>context?
> >> >> >
> >> >> >Thanks,
> >> >> >Om
> >> >> I looked at the commit diffs again.  I missed seeing earlier that you
> >> >>want
> >> >> to propagate rolloverindex to the List's API surface in order to
> >> >> communicate that information to the DataGrid.  I was thinking that
> >>there
> >> >> can be some other List that represents the List that is a column in
> >>the
> >> >> DG.  In theory it can be built out of most of the same pieces as
> >>List.
> >> >>
> >> >
> >> >I wanted to do that anyways.  Right now, when there is more than a
> >>page of
> >> >data, three scrollbars (for 3 columns) show up.  I wanted to create a
> >> >pared
> >> >down version of ListView that takes in an optional ScrollBar.  And
> >>allow
> >> >the DataGridView to have a scrollbar.  Any thoughts on that?
> >> Does the ScrollBar need to be optional for ListView?  IMO, the custom
> >> ListView wouldn't have a scrollbar at all.
> >>
> >
> >(Option 1:)
> >I thought the ScrollBar could be a view bead that we can inject into
> >ListView as needed.
> >
> >(Option 2:)
> >Or do you mean that we create a new DataGridColumnListView with everything
> >in ListView except for ScrollBar and add RollOver behavior like you
> >mentioned earlier?
> >
> >Option 2 means that we are not DRY.  With option 1, we can use a ListView
> >in mobile as well where there is no need for a visible scrollbar.
>
> I think Option 2 is best or the ListView has its Scrollbar removed and if
> you want a scrollable list you add that as a bead to the List strand. We
> could then package a ScrollableList which would probably be the one most
> people use.
>

Your phrasing is a bit confusing.  Are you proposing an option 3: "ListView
has its Scrollbar removed and if
you want a scrollable list you add that as a bead to the List strand. " ?

In that case, I like option 3.  If I understand correct, it is like option1
except you want the scrollbar bead to be added to the List and not
ListView.

Thanks,
Om



>
> With the scrollbar out of the picture, the DataGrid could use a scrolling
> layout on the Container that houses the individual lists that make up the
> columns.
>
> Getting virtualization might be tricker in any case.
> --peter
>
> >
> >
> >
> >> >
> >> >
> >> >>This sounds good to me.  I will give this a shot and see how things
> >>look.
> >> >Thanks for the pseudo code, makes it clearer to me.
> >> OK, good luck.
> >>
> >> -Alex
> >>
> >>
>
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Peter Ent <pe...@adobe.com>.

On 11/14/13 2:55 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>On Thu, Nov 14, 2013 at 11:45 AM, Alex Harui <ah...@adobe.com> wrote:
>
>>
>>
>> On 11/14/13 10:57 AM, "OmPrakash Muppirala" <bi...@gmail.com>
>>wrote:
>>
>> >On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com> wrote:
>> >
>> >>
>> >>
>> >> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com>
>> wrote:
>> >>
>> >> >I think I get it, but I guess I will need some pseudo code to
>> >>understand
>> >> >completely.  What exactly does 'hang it on a strand' man in this
>> >>context?
>> >> >
>> >> >Thanks,
>> >> >Om
>> >> I looked at the commit diffs again.  I missed seeing earlier that you
>> >>want
>> >> to propagate rolloverindex to the List's API surface in order to
>> >> communicate that information to the DataGrid.  I was thinking that
>>there
>> >> can be some other List that represents the List that is a column in
>>the
>> >> DG.  In theory it can be built out of most of the same pieces as
>>List.
>> >>
>> >
>> >I wanted to do that anyways.  Right now, when there is more than a
>>page of
>> >data, three scrollbars (for 3 columns) show up.  I wanted to create a
>> >pared
>> >down version of ListView that takes in an optional ScrollBar.  And
>>allow
>> >the DataGridView to have a scrollbar.  Any thoughts on that?
>> Does the ScrollBar need to be optional for ListView?  IMO, the custom
>> ListView wouldn't have a scrollbar at all.
>>
>
>(Option 1:)
>I thought the ScrollBar could be a view bead that we can inject into
>ListView as needed.
>
>(Option 2:)
>Or do you mean that we create a new DataGridColumnListView with everything
>in ListView except for ScrollBar and add RollOver behavior like you
>mentioned earlier?
>
>Option 2 means that we are not DRY.  With option 1, we can use a ListView
>in mobile as well where there is no need for a visible scrollbar.

I think Option 2 is best or the ListView has its Scrollbar removed and if
you want a scrollable list you add that as a bead to the List strand. We
could then package a ScrollableList which would probably be the one most
people use.

With the scrollbar out of the picture, the DataGrid could use a scrolling
layout on the Container that houses the individual lists that make up the
columns.

Getting virtualization might be tricker in any case.
--peter

>
>
>
>> >
>> >
>> >>This sounds good to me.  I will give this a shot and see how things
>>look.
>> >Thanks for the pseudo code, makes it clearer to me.
>> OK, good luck.
>>
>> -Alex
>>
>>


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Thu, Nov 14, 2013 at 11:45 AM, Alex Harui <ah...@adobe.com> wrote:

>
>
> On 11/14/13 10:57 AM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
> >On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com> wrote:
> >
> >>
> >>
> >> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com>
> wrote:
> >>
> >> >I think I get it, but I guess I will need some pseudo code to
> >>understand
> >> >completely.  What exactly does 'hang it on a strand' man in this
> >>context?
> >> >
> >> >Thanks,
> >> >Om
> >> I looked at the commit diffs again.  I missed seeing earlier that you
> >>want
> >> to propagate rolloverindex to the List's API surface in order to
> >> communicate that information to the DataGrid.  I was thinking that there
> >> can be some other List that represents the List that is a column in the
> >> DG.  In theory it can be built out of most of the same pieces as List.
> >>
> >
> >I wanted to do that anyways.  Right now, when there is more than a page of
> >data, three scrollbars (for 3 columns) show up.  I wanted to create a
> >pared
> >down version of ListView that takes in an optional ScrollBar.  And allow
> >the DataGridView to have a scrollbar.  Any thoughts on that?
> Does the ScrollBar need to be optional for ListView?  IMO, the custom
> ListView wouldn't have a scrollbar at all.
>

(Option 1:)
I thought the ScrollBar could be a view bead that we can inject into
ListView as needed.

(Option 2:)
Or do you mean that we create a new DataGridColumnListView with everything
in ListView except for ScrollBar and add RollOver behavior like you
mentioned earlier?

Option 2 means that we are not DRY.  With option 1, we can use a ListView
in mobile as well where there is no need for a visible scrollbar.




> >
> >
> >>This sounds good to me.  I will give this a shot and see how things look.
> >Thanks for the pseudo code, makes it clearer to me.
> OK, good luck.
>
> -Alex
>
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.

On 11/14/13 10:57 AM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com> wrote:
>
>>
>>
>> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>>
>> >I think I get it, but I guess I will need some pseudo code to
>>understand
>> >completely.  What exactly does 'hang it on a strand' man in this
>>context?
>> >
>> >Thanks,
>> >Om
>> I looked at the commit diffs again.  I missed seeing earlier that you
>>want
>> to propagate rolloverindex to the List's API surface in order to
>> communicate that information to the DataGrid.  I was thinking that there
>> can be some other List that represents the List that is a column in the
>> DG.  In theory it can be built out of most of the same pieces as List.
>>
>
>I wanted to do that anyways.  Right now, when there is more than a page of
>data, three scrollbars (for 3 columns) show up.  I wanted to create a
>pared
>down version of ListView that takes in an optional ScrollBar.  And allow
>the DataGridView to have a scrollbar.  Any thoughts on that?
Does the ScrollBar need to be optional for ListView?  IMO, the custom
ListView wouldn't have a scrollbar at all.
>
>
>>This sounds good to me.  I will give this a shot and see how things look.
>Thanks for the pseudo code, makes it clearer to me.
OK, good luck.

-Alex


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Wed, Nov 13, 2013 at 9:12 PM, Alex Harui <ah...@adobe.com> wrote:

>
>
> On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
> >I think I get it, but I guess I will need some pseudo code to understand
> >completely.  What exactly does 'hang it on a strand' man in this context?
> >
> >Thanks,
> >Om
> I looked at the commit diffs again.  I missed seeing earlier that you want
> to propagate rolloverindex to the List's API surface in order to
> communicate that information to the DataGrid.  I was thinking that there
> can be some other List that represents the List that is a column in the
> DG.  In theory it can be built out of most of the same pieces as List.
>

I wanted to do that anyways.  Right now, when there is more than a page of
data, three scrollbars (for 3 columns) show up.  I wanted to create a pared
down version of ListView that takes in an optional ScrollBar.  And allow
the DataGridView to have a scrollbar.  Any thoughts on that?


> Let's call it DataGridColumnList.  That way the List that people stick in
> the UI isn't sporting a rollOverIndex property that won't be useful to
> them.  DataGridColumnList extends UIBase like List but it would have a
> different IRollOverBead, one that knows that its strand, which is a
> DataGridColumnList, has a parent DataGrid which has a strand which has an
> IRollOverModel on its strand, but it isn't the beadModel property.
>
> So, then I think the pieces are:
>
> Class RollOverModel implements IRollOverModel
> {
>    Public function get/set rollOverIndex
> }
>
> Class DataGridColumnList extends UIBase
> {
>    Public function get dataGrid():DataGrid
>    {
>         return parent; // or whatever
>    }
> }
>
> /* rollover impl for List */
> Class RollOverBead implements IRollOverBead
> {
>   Var rollOverModel:IRollOverModel;
>
>   Public function set strand(strand:IStrand):void
>   {
>         rollOverModel = strand.getBeadByType(IRollOverModel);
>         addEventListener("rollOver", rollOverHandler);
>   }
>
>   Function rollOverHandler()
>   {
>      // set/unset hovered on renderer
>      // draw rect highlight
>   }
> }
>
> /* rollover impl for DataGridColumnList */
> Class DGColumnListRollOverBead implements IRollOverBead
> {
>   Var rollOverModel:IRollOverModel;
>
>   Public function set strand(strand:IStrand):void
>   {
>         rollOverModel = strand.dataGrid.getBeadByType(IRollOverModel);
>         addEventListener("rollOver", rollOverHandler);
>   }
>
>
>   Function rollOverHandler()
>   {
>       // set/unset hovered on renderer
>       // does not draw rect highlight
>   }
>
>
> }
>
> /* rollover impl for DataGrid */
> Class DGRollOverBead implements IRollOverBead
> {
>   Var rollOverModel:IRollOverModel;
>
>   Public function set strand(strand:IStrand):void
>   {
>         rollOverModel = strand.getBeadByType(IRollOverModel);
>         addEventListener("rollOver", rollOverHandler);
>   }
>
>   Function rollOverHandler()
>   {
>       // Draws rect highlight across all columns
>   }
> }
>
>
>
> Then the CSS:
>
> List
> {
>   IRollOverBead: ClassReference(RollOverBead);
>   IRollOverModel: ClassReference(RollOverModel);
> }
>
> DataGridColumnList
> {
>   IRollOverBead: ClassReference(DGColumnListRollOverBead);
> }
>
> DataGrid
> {
>   IRollOverBead: ClassReference(DGRollOverBead);
>   IRollOverModel: ClassReference(RollOverModel);
> }
>
>
This sounds good to me.  I will give this a shot and see how things look.
Thanks for the pseudo code, makes it clearer to me.



> Of course, I could still be wrong...
>
>
Damn, I wish I saw this line first before reading the whole email ;-)

Thanks,
Om


>  -Alex
>
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.

On 11/13/13 8:26 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>I think I get it, but I guess I will need some pseudo code to understand
>completely.  What exactly does 'hang it on a strand' man in this context?
>
>Thanks,
>Om
I looked at the commit diffs again.  I missed seeing earlier that you want
to propagate rolloverindex to the List's API surface in order to
communicate that information to the DataGrid.  I was thinking that there
can be some other List that represents the List that is a column in the
DG.  In theory it can be built out of most of the same pieces as List.
Let's call it DataGridColumnList.  That way the List that people stick in
the UI isn't sporting a rollOverIndex property that won't be useful to
them.  DataGridColumnList extends UIBase like List but it would have a
different IRollOverBead, one that knows that its strand, which is a
DataGridColumnList, has a parent DataGrid which has a strand which has an
IRollOverModel on its strand, but it isn't the beadModel property.

So, then I think the pieces are:

Class RollOverModel implements IRollOverModel
{
   Public function get/set rollOverIndex
}

Class DataGridColumnList extends UIBase
{
   Public function get dataGrid():DataGrid
   {
	return parent; // or whatever
   }
}

/* rollover impl for List */
Class RollOverBead implements IRollOverBead
{
  Var rollOverModel:IRollOverModel;

  Public function set strand(strand:IStrand):void
  {
	rollOverModel = strand.getBeadByType(IRollOverModel);
        addEventListener("rollOver", rollOverHandler);	
  }

  Function rollOverHandler()
  {
     // set/unset hovered on renderer
     // draw rect highlight
  }
}

/* rollover impl for DataGridColumnList */
Class DGColumnListRollOverBead implements IRollOverBead
{
  Var rollOverModel:IRollOverModel;

  Public function set strand(strand:IStrand):void
  {
	rollOverModel = strand.dataGrid.getBeadByType(IRollOverModel);
	addEventListener("rollOver", rollOverHandler);	
  }


  Function rollOverHandler()
  {
      // set/unset hovered on renderer
      // does not draw rect highlight
  }


}

/* rollover impl for DataGrid */
Class DGRollOverBead implements IRollOverBead
{
  Var rollOverModel:IRollOverModel;

  Public function set strand(strand:IStrand):void
  {
	rollOverModel = strand.getBeadByType(IRollOverModel);
	addEventListener("rollOver", rollOverHandler);
  }

  Function rollOverHandler()
  {
      // Draws rect highlight across all columns
  }
}



Then the CSS:

List
{
  IRollOverBead: ClassReference(RollOverBead);
  IRollOverModel: ClassReference(RollOverModel);
}

DataGridColumnList
{
  IRollOverBead: ClassReference(DGColumnListRollOverBead);
}

DataGrid
{
  IRollOverBead: ClassReference(DGRollOverBead);
  IRollOverModel: ClassReference(RollOverModel);
}

Of course, I could still be wrong...

-Alex


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Nov 13, 2013 8:05 PM, "Alex Harui" <ah...@adobe.com> wrote:
>
>
>
> On 11/13/13 1:33 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
> >
> >The usecase here is that there are n Lists (for each column) composed by
> >the DataGrid component.  If the users rollsover an on any list, the
> >corresponding itemrenderers in each column must highlight themselves.
 So,
> >keeping track of the current rollOverIndex as part of DataGrid's model is
> >a
> >must.
> >
> >After reading your comments multiple times, here is one approach we could
> >do.
>
> I guess I'm not understanding why you want to combine the rollovermodel
> with the main model.  You are certainly allowed to do so in order to
> "inline" something for speed, but IMO, it is important to make sure that
> you have the right separation of concerns which means that a set of beads
> that implements rollover can be added or removed as a set and everything
> else keeps working.  Theoretically, you should be able to set up media
> queries such that when this app gets loaded in an Android browser it never
> loads the rollover implementation.  You don't always get to re-compile for
> the target.
>
> In my mind, the rolloverindex isn't part of the API surface of the
> component.  It should be ok to have a separate rollovermodel and hang it
> on the strand and have an implementation bead find it via getBeadByType.

I think I get it, but I guess I will need some pseudo code to understand
completely.  What exactly does 'hang it on a strand' man in this context?

Thanks,
Om

>
> But of course, I could be wrong.  I think this is a useful discussion.
>
> Thanks,
> -Alex
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.

On 11/13/13 1:33 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
>The usecase here is that there are n Lists (for each column) composed by
>the DataGrid component.  If the users rollsover an on any list, the
>corresponding itemrenderers in each column must highlight themselves.  So,
>keeping track of the current rollOverIndex as part of DataGrid's model is
>a
>must.
>
>After reading your comments multiple times, here is one approach we could
>do.

I guess I'm not understanding why you want to combine the rollovermodel
with the main model.  You are certainly allowed to do so in order to
"inline" something for speed, but IMO, it is important to make sure that
you have the right separation of concerns which means that a set of beads
that implements rollover can be added or removed as a set and everything
else keeps working.  Theoretically, you should be able to set up media
queries such that when this app gets loaded in an Android browser it never
loads the rollover implementation.  You don't always get to re-compile for
the target.

In my mind, the rolloverindex isn't part of the API surface of the
component.  It should be ok to have a separate rollovermodel and hang it
on the strand and have an implementation bead find it via getBeadByType.

But of course, I could be wrong.  I think this is a useful discussion.

Thanks,
-Alex


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Wed, Nov 13, 2013 at 11:37 AM, Alex Harui <ah...@adobe.com> wrote:

>
>
> On 11/13/13 8:17 AM, "Peter Ent" <pe...@adobe.com> wrote:
>
> >
> >
> >On 11/13/13 3:19 AM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
> >
> >>On Tue, Nov 12, 2013 at 8:27 PM, Alex Harui <ah...@adobe.com> wrote:
> >>
> >>> Hi Om,
> >>>
> >>> Looks good.  Some points to ponder:
> >>>
> >>> 1) Ideally, in a Pay-as-you-go philosophy, a rolloverIndex would not be
> >>>in
> >>> the ArraySelectionModel since, in theory, ArraySelectionModel should be
> >>> reusable in mobile devices where there is no rollover.  I honestly
> >>>don't
> >>> know what the right answer is.  There are options like having a
> >>>subclass
> >>> of ArraySelectionModel that supports rollover.  There is the option of
> >>> having rolloverIndex stored somewhere else.  Maybe rollover is a
> >>> presentation concept, or maybe rollover should be entirely encapsulated
> >>>in
> >>> its own bead so it can be dropped out of mobile configs?  But I think
> >>>the
> >>> entire mousecontroller will be replaced by a touchcontroller in that
> >>>case.
> >>>  Another thing to consider is that, if a JS DG happens to wrap a
> >>> third-party DG or I think there is one built into HTML5, and the JS
> >>> version doesn't support rollover, how can we extract rollover from the
> >>>AS
> >>> version?
> >>>
> >>
> >>Yes, this was bothering me as well.  I couldnt think of a great place to
> >>put this info.  Since it is not possible to have multiple models for a
> >>component, having a subclass of ArraySelectionModel called
> >>ArraySelectionAndRollOverModel would probably work.  But it will soon
> >>start
> >>getting unwieldy if go this route.
> Agreed
> >>
> >>The other option is to change ArraySelectionModel to
> >>ArrayInteractionsModel
> >>and have an array of composed models, ex. SelectionModel, RollOverModel,
> >>TouchModel, etc.  This way, we can string as many models as needed on a
> >>pay-as-you-go basis.
> >>
> >>What do you think?
> >
> >My thought on roll-over was to let the mouse controller dispatch the event
> >on the strand and any itemRenderers that were interested in handling
> >roll-over would listen for it and implement it themselves. That doesn't
> >necessarily lead to a unified look to something like the DataGrid, but it
> >supports pay-as-you go. That is, on mobile devices the mouse controller
> >would be replaced with a touch controller and no roll-over events would be
> >issued (unless you have a new device that detects 'hover' or hand-waving
> >or something in the future). If you leave the itemRenderers the same in
> >the mobile version as the desktop version, the roll-over event listeners
> >are harmless. The grid presentation model could contain the feedback color
> >for handling roll-over and the itemRenderers could access that and use it
> >to provide a uniform look.
> That's another viable implementation.  It has the drawback that the
> renderers carry rollover code into environments where it isn't needed, but
> the renderers rollover implementation could also be a bead.
>
> I can think of several options:
> 1) Your sugestion of array of composed models
> 2) simply add another bead of type IRollOverModel or something like that
> that a rollOver implementation looks for
> 3) bake the rolloverindex into the rollover bead
> 4) Go heirarchical: The rollover bead has its own model.
>
> To me, the primary model for a component is the data model which
> represents the public properties for the component (the attributes that
> would be set from MXML and/or properties most often checked by event
> handlers and other code).  The internal workings of a component can have
> its own data to share, but they don't have to be separate models, the
> point of assignable models is so that richer views can work with richer
> models.  Other internal feature implementations only need to have some
> agreement between the beads that it is comprised of.
>
> The simplest rollover implementation that just slaps a highlight rectangle
> on a renderer doesn't need a separate model at all.  It can probably be
> self-contained.  However, the renderer wouldn't get a choice of altering
> its visuals to react to the highlight, like changing font color to
> contrast better against the highlight.
>
> So, the renderers probably do need to know if they are highlighted.  Some
> implementations may share a rollOverIndex, others may not and simply set
> the hovered property on a renderer or otherwise alter the calculation of a
> "currentState" property.
>
> So right now, I would say that rollover isn't really part of selection and
> doesn't nee d to be composed into the main model.  And hanging in the
> central rollover bead or having that bead have its own model is sufficient.
>
> >>
> >>
> >>>
> >>> 2) There is an entirely alternate implementation of rollover where it
> >>>is
> >>> completely delegated to the renderer to determine based on mouse events
> >>> what its display/rollover and selection state is.  Think of renderers
> >>>that
> >>> have buttons in them for deleting that row.  Clicking on the button may
> >>>or
> >>> may not change the selection, rollover the button may not cause
> >>>rollover
> >>> highlighting of the row, just the button.  I'm not saying this a
> >>>preferred
> >>> implementation, just that we want to make sure we make as many other
> >>>beads
> >>> reusable as possible in such an implementation, and not lock folks into
> >>> one implementation or another.
> >>>
> >>
> >>So, each renderer has selection, rollover, etc. models?
> Maybe as a model or maybe just as a current state.  MX renderers had to
> ask the containing list if they were highlighted or not.  Spark renderers
> have a hovered property that affects getCurrentRendererState.
>
> -Alex
>
>

The usecase here is that there are n Lists (for each column) composed by
the DataGrid component.  If the users rollsover an on any list, the
corresponding itemrenderers in each column must highlight themselves.  So,
keeping track of the current rollOverIndex as part of DataGrid's model is a
must.

After reading your comments multiple times, here is one approach we could
do.

default.css:
List
{
    IBeadModel:
ClassReference("org.apache.flex.html.staticControls.beads.models.ListInteractionModel");
    //other stuff
}

public class ListInteractionModel extends ArrayInteractionModel implements
ISelectionModel, IRollOverModel
{
    private var selectionModel:ISelectionModel = new SelectionModel();
    //Implements ISelectionModel (proxies selectionModel)
    function get selectedIndex():int;
    function set selectedIndex(value:int):void;
    function get selectedItem():Object;
    function set selectedItem(value:Object):void;

    private var rollOverModel:IRollOverModel = new RollOverModel();
    //Implements IRollOverModel
    function get rollOverIndex():int;
    function set rollOverIndex(value:int):void;
}

public class ArrayInteractionModel extends EventDispatcher implements
IArrayInteractionModel
{
    //Implements IArrayInteractionModel
    function get dataProvider():Object;
    function set dataProvider(value:Object):void;

    //Implements IBeadModel
    function set strand(value:IStrand):void
}

public interface IArrayInteractionModel extends IBeadModel
{
    function get dataProvider():Object;
    function set dataProvider(value:Object):void;
}

So, if we have a mobile list, we would define its model this way:

public class MobileListInteractionModel extends ArrayInteractionModel
implements ISelectionModel
{
    private var selectionModel:ISelectionModel = new SelectionModel();
    //Implements ISelectionModel (proxies selectionModel)
    function get selectedIndex():int;
    function set selectedIndex(value:int):void;
    function get selectedItem():Object;
    function set selectedItem(value:Object):void;
}

What do you think?

Thanks,
Om

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.

On 11/13/13 8:17 AM, "Peter Ent" <pe...@adobe.com> wrote:

>
>
>On 11/13/13 3:19 AM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
>>On Tue, Nov 12, 2013 at 8:27 PM, Alex Harui <ah...@adobe.com> wrote:
>>
>>> Hi Om,
>>>
>>> Looks good.  Some points to ponder:
>>>
>>> 1) Ideally, in a Pay-as-you-go philosophy, a rolloverIndex would not be
>>>in
>>> the ArraySelectionModel since, in theory, ArraySelectionModel should be
>>> reusable in mobile devices where there is no rollover.  I honestly
>>>don't
>>> know what the right answer is.  There are options like having a
>>>subclass
>>> of ArraySelectionModel that supports rollover.  There is the option of
>>> having rolloverIndex stored somewhere else.  Maybe rollover is a
>>> presentation concept, or maybe rollover should be entirely encapsulated
>>>in
>>> its own bead so it can be dropped out of mobile configs?  But I think
>>>the
>>> entire mousecontroller will be replaced by a touchcontroller in that
>>>case.
>>>  Another thing to consider is that, if a JS DG happens to wrap a
>>> third-party DG or I think there is one built into HTML5, and the JS
>>> version doesn't support rollover, how can we extract rollover from the
>>>AS
>>> version?
>>>
>>
>>Yes, this was bothering me as well.  I couldnt think of a great place to
>>put this info.  Since it is not possible to have multiple models for a
>>component, having a subclass of ArraySelectionModel called
>>ArraySelectionAndRollOverModel would probably work.  But it will soon
>>start
>>getting unwieldy if go this route.
Agreed
>>
>>The other option is to change ArraySelectionModel to
>>ArrayInteractionsModel
>>and have an array of composed models, ex. SelectionModel, RollOverModel,
>>TouchModel, etc.  This way, we can string as many models as needed on a
>>pay-as-you-go basis.
>>
>>What do you think?
>
>My thought on roll-over was to let the mouse controller dispatch the event
>on the strand and any itemRenderers that were interested in handling
>roll-over would listen for it and implement it themselves. That doesn't
>necessarily lead to a unified look to something like the DataGrid, but it
>supports pay-as-you go. That is, on mobile devices the mouse controller
>would be replaced with a touch controller and no roll-over events would be
>issued (unless you have a new device that detects 'hover' or hand-waving
>or something in the future). If you leave the itemRenderers the same in
>the mobile version as the desktop version, the roll-over event listeners
>are harmless. The grid presentation model could contain the feedback color
>for handling roll-over and the itemRenderers could access that and use it
>to provide a uniform look.
That's another viable implementation.  It has the drawback that the
renderers carry rollover code into environments where it isn't needed, but
the renderers rollover implementation could also be a bead.

I can think of several options:
1) Your sugestion of array of composed models
2) simply add another bead of type IRollOverModel or something like that
that a rollOver implementation looks for
3) bake the rolloverindex into the rollover bead
4) Go heirarchical: The rollover bead has its own model.

To me, the primary model for a component is the data model which
represents the public properties for the component (the attributes that
would be set from MXML and/or properties most often checked by event
handlers and other code).  The internal workings of a component can have
its own data to share, but they don't have to be separate models, the
point of assignable models is so that richer views can work with richer
models.  Other internal feature implementations only need to have some
agreement between the beads that it is comprised of.

The simplest rollover implementation that just slaps a highlight rectangle
on a renderer doesn't need a separate model at all.  It can probably be
self-contained.  However, the renderer wouldn't get a choice of altering
its visuals to react to the highlight, like changing font color to
contrast better against the highlight.

So, the renderers probably do need to know if they are highlighted.  Some
implementations may share a rollOverIndex, others may not and simply set
the hovered property on a renderer or otherwise alter the calculation of a
"currentState" property.

So right now, I would say that rollover isn't really part of selection and
doesn't nee d to be composed into the main model.  And hanging in the
central rollover bead or having that bead have its own model is sufficient.

>>
>>
>>>
>>> 2) There is an entirely alternate implementation of rollover where it
>>>is
>>> completely delegated to the renderer to determine based on mouse events
>>> what its display/rollover and selection state is.  Think of renderers
>>>that
>>> have buttons in them for deleting that row.  Clicking on the button may
>>>or
>>> may not change the selection, rollover the button may not cause
>>>rollover
>>> highlighting of the row, just the button.  I'm not saying this a
>>>preferred
>>> implementation, just that we want to make sure we make as many other
>>>beads
>>> reusable as possible in such an implementation, and not lock folks into
>>> one implementation or another.
>>>
>>
>>So, each renderer has selection, rollover, etc. models?
Maybe as a model or maybe just as a current state.  MX renderers had to
ask the containing list if they were highlighted or not.  Spark renderers
have a hovered property that affects getCurrentRendererState.

-Alex


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Peter Ent <pe...@adobe.com>.

On 11/13/13 3:19 AM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>On Tue, Nov 12, 2013 at 8:27 PM, Alex Harui <ah...@adobe.com> wrote:
>
>> Hi Om,
>>
>> Looks good.  Some points to ponder:
>>
>> 1) Ideally, in a Pay-as-you-go philosophy, a rolloverIndex would not be
>>in
>> the ArraySelectionModel since, in theory, ArraySelectionModel should be
>> reusable in mobile devices where there is no rollover.  I honestly don't
>> know what the right answer is.  There are options like having a subclass
>> of ArraySelectionModel that supports rollover.  There is the option of
>> having rolloverIndex stored somewhere else.  Maybe rollover is a
>> presentation concept, or maybe rollover should be entirely encapsulated
>>in
>> its own bead so it can be dropped out of mobile configs?  But I think
>>the
>> entire mousecontroller will be replaced by a touchcontroller in that
>>case.
>>  Another thing to consider is that, if a JS DG happens to wrap a
>> third-party DG or I think there is one built into HTML5, and the JS
>> version doesn't support rollover, how can we extract rollover from the
>>AS
>> version?
>>
>
>Yes, this was bothering me as well.  I couldnt think of a great place to
>put this info.  Since it is not possible to have multiple models for a
>component, having a subclass of ArraySelectionModel called
>ArraySelectionAndRollOverModel would probably work.  But it will soon
>start
>getting unwieldy if go this route.
>
>The other option is to change ArraySelectionModel to
>ArrayInteractionsModel
>and have an array of composed models, ex. SelectionModel, RollOverModel,
>TouchModel, etc.  This way, we can string as many models as needed on a
>pay-as-you-go basis.
>
>What do you think?

My thought on roll-over was to let the mouse controller dispatch the event
on the strand and any itemRenderers that were interested in handling
roll-over would listen for it and implement it themselves. That doesn't
necessarily lead to a unified look to something like the DataGrid, but it
supports pay-as-you go. That is, on mobile devices the mouse controller
would be replaced with a touch controller and no roll-over events would be
issued (unless you have a new device that detects 'hover' or hand-waving
or something in the future). If you leave the itemRenderers the same in
the mobile version as the desktop version, the roll-over event listeners
are harmless. The grid presentation model could contain the feedback color
for handling roll-over and the itemRenderers could access that and use it
to provide a uniform look.

As an application writer I've never had a need to know what row was being
roll-over/out except to know to highlight it and the event handling in the
itemRenderer would suffice. If I did need such a thing, then I would
create my own model, probably extending ArraySelectionModel, and assign
that model to the DataGrid. In other words, I see this more as something
that application writers would provide rather than being provided by the
framework except for the event dispatching my the mouse or mobile
controllers and handled only by the itemRenderers should they be written
to do so.
--peter
>
>
>
>>
>> 2) There is an entirely alternate implementation of rollover where it is
>> completely delegated to the renderer to determine based on mouse events
>> what its display/rollover and selection state is.  Think of renderers
>>that
>> have buttons in them for deleting that row.  Clicking on the button may
>>or
>> may not change the selection, rollover the button may not cause rollover
>> highlighting of the row, just the button.  I'm not saying this a
>>preferred
>> implementation, just that we want to make sure we make as many other
>>beads
>> reusable as possible in such an implementation, and not lock folks into
>> one implementation or another.
>>
>
>So, each renderer has selection, rollover, etc. models?
>
>Thanks,
>Om
>
>
>>
>> Anyway, that last point is mainly food for thought.  From what I could
>> tell from the diff this is a perfectly fine implementation of rollover,
>> the only thing that stood out was adding rolloverindex to
>> ArraySelectionModel.  Any thoughts on where else to hang it?  Of course,
>> there is the argument that it is just one property slot, but I can
>>promise
>> you I've seen that philosophy cause slow but steady size and dependency
>> creep.
>>
>> Thanks,
>> -Alex
>>
>> On 11/12/13 6:09 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>>
>> >This is my first real contribution to the FlexJS project.  Alex/Peter
>>can
>> >you please review this and let me know if I am on the right track?
>> >Once you approve, I will start looking at the JS version of the same
>> >logic.
>> >
>> >Thanks,
>> >Om
>> >
>> >
>> >On Tue, Nov 12, 2013 at 6:07 PM, <bi...@apache.org> wrote:
>> >
>> >> Updated Branches:
>> >>   refs/heads/develop 22f891823 -> 67c11796e
>> >>
>> >>
>> >> Add functionality to AS version of DataGrid to highlight entire row
>>when
>> >> rolling over any column
>> >> JS version to follow after discussion on dev@f.a.o
>> >>
>> >>
>> >> Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
>> >> Commit:
>> http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/67c11796
>> >> Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/67c11796
>> >> Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/67c11796
>> >>
>> >> Branch: refs/heads/develop
>> >> Commit: 67c11796e271bf7811e8afb12f556aa2386d2960
>> >> Parents: 22f8918
>> >> Author: Om <bi...@gmail.com>
>> >> Authored: Tue Nov 12 18:05:48 2013 -0800
>> >> Committer: Om <bi...@gmail.com>
>> >> Committed: Tue Nov 12 18:06:26 2013 -0800
>> >>
>> >>
>>----------------------------------------------------------------------
>> >>  .../src/org/apache/flex/core/IRollOverModel.as  | 28
>> >>++++++++++++++++++++
>> >>  .../org/apache/flex/html/staticControls/List.as | 12 ++++++++-
>> >>  .../html/staticControls/beads/DataGridView.as   | 13 +++++++++
>> >>  .../flex/html/staticControls/beads/ListView.as  | 19 +++++++++++++
>> >>  .../controllers/ItemRendererMouseController.as  |  1 +
>> >>  .../ListSingleSelectionMouseController.as       | 10 ++++++-
>> >>  .../beads/models/ArraySelectionModel.as         | 14 +++++++++-
>> >>  7 files changed, 94 insertions(+), 3 deletions(-)
>> >>
>>----------------------------------------------------------------------
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>> >>/as/src/org/apache/flex/core/IRollOverModel.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git a/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> >> b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> >> new file mode 100644
>> >> index 0000000..d38aa41
>> >> --- /dev/null
>> >> +++ b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> >> @@ -0,0 +1,28 @@
>> >>
>> >>
>>
>>>>+//////////////////////////////////////////////////////////////////////
>>>>//
>> >>////////
>> >> +//
>> >> +//  Licensed to the Apache Software Foundation (ASF) under one or
>>more
>> >> +//  contributor license agreements.  See the NOTICE file distributed
>> >>with
>> >> +//  this work for additional information regarding copyright
>>ownership.
>> >> +//  The ASF licenses this file to You under the Apache License,
>>Version
>> >> 2.0
>> >> +//  (the "License"); you may not use this file except in compliance
>> >>with
>> >> +//  the License.  You may obtain a copy of the License at
>> >> +//
>> >> +//      http://www.apache.org/licenses/LICENSE-2.0
>> >> +//
>> >> +//  Unless required by applicable law or agreed to in writing,
>>software
>> >> +//  distributed under the License is distributed on an "AS IS"
>>BASIS,
>> >> +//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> >> implied.
>> >> +//  See the License for the specific language governing permissions
>>and
>> >> +//  limitations under the License.
>> >> +//
>> >>
>> >>
>>
>>>>+//////////////////////////////////////////////////////////////////////
>>>>//
>> >>////////
>> >> +package org.apache.flex.core
>> >> +{
>> >> +       import org.apache.flex.events.IEventDispatcher;
>> >> +
>> >> +       public interface IRollOverModel extends IEventDispatcher,
>> >> IBeadModel
>> >> +       {
>> >> +               function get rollOverIndex():int;
>> >> +               function set rollOverIndex(value:int):void;
>> >> +       }
>> >> +}
>> >> \ No newline at end of file
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>> >>/as/src/org/apache/flex/html/staticControls/List.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> >> b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> >> index 7cb3b6d..da1638e 100644
>> >> --- a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> >> +++ b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> >> @@ -18,8 +18,9 @@
>> >>
>> >>
>>
>>>>///////////////////////////////////////////////////////////////////////
>>>>//
>> >>///////
>> >>  package org.apache.flex.html.staticControls
>> >>  {
>> >> +       import org.apache.flex.core.IRollOverModel;
>> >>         import org.apache.flex.core.ISelectionModel;
>> >> -    import org.apache.flex.core.UIBase;
>> >> +       import org.apache.flex.core.UIBase;
>> >>         import org.apache.flex.core.ValuesManager;
>> >>         import
>> >>
>>
>>>>org.apache.flex.html.staticControls.beads.IDataProviderItemRendererMapp
>>>>er
>> >>;
>> >>
>> >> @@ -56,6 +57,15 @@ package org.apache.flex.html.staticControls
>> >>                 {
>> >>                         ISelectionModel(model).selectedIndex = value;
>> >>                 }
>> >> +
>> >> +        public function get rollOverIndex():int
>> >> +               {
>> >> +                       return IRollOverModel(model).rollOverIndex;
>> >> +               }
>> >> +               public function set rollOverIndex(value:int):void
>> >> +               {
>> >> +                       IRollOverModel(model).rollOverIndex = value;
>> >> +               }
>> >>
>> >>                 public function get selectedItem():Object
>> >>                 {
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>> >>/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridV
>>>>ie
>> >>w.as
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridV
>>>>ie
>> >>w.as
>> >> index 4088734..f2d3cc2 100644
>> >> ---
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridV
>>>>ie
>> >>w.as
>> >> +++
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridV
>>>>ie
>> >>w.as
>> >> @@ -85,6 +85,7 @@ package org.apache.flex.html.staticControls.beads
>> >>                                 columns.push(column);
>> >>
>> >>
>> >> column.addEventListener('change',columnListChangeHandler);
>> >> +
>> >> column.addEventListener('rollover',columnListRollOverHandler);
>> >>                         }
>> >>
>> >>
>> >>
>>
>>>>IEventDispatcher(_strand).addEventListener("widthChanged",handleSizeCha
>>>>ng
>> >>e);
>> >> @@ -139,5 +140,17 @@ package
>>org.apache.flex.html.staticControls.beads
>> >>
>> >>                         IEventDispatcher(_strand).dispatchEvent(new
>> >> Event('change'));
>> >>                 }
>> >> +
>> >> +               private function
>> >> columnListRollOverHandler(event:Event):void
>> >> +               {
>> >> +                       var list:List = event.target as List;
>> >> +                       for(var i:int=0; i < columns.length; i++) {
>> >> +                               if (list != columns[i]) {
>> >> +                                       columns[i].rollOverIndex =
>> >> list.rollOverIndex;
>> >> +                               }
>> >> +                       }
>> >> +
>> >> +                       IEventDispatcher(_strand).dispatchEvent(new
>> >> Event('rollOver'));
>> >> +               }
>> >>         }
>> >>  }
>> >> \ No newline at end of file
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>> >>/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.
>>>>as
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.
>>>>as
>> >> index dbe9e36..b8c9ab9 100644
>> >> ---
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.
>>>>as
>> >> +++
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.
>>>>as
>> >> @@ -27,6 +27,7 @@ package org.apache.flex.html.staticControls.beads
>> >>         import org.apache.flex.core.IItemRendererParent;
>> >>         import org.apache.flex.core.ILayoutParent;
>> >>         import org.apache.flex.core.IParent;
>> >> +       import org.apache.flex.core.IRollOverModel;
>> >>         import org.apache.flex.core.ISelectionModel;
>> >>         import org.apache.flex.core.IStrand;
>> >>         import org.apache.flex.core.Strand;
>> >> @@ -96,6 +97,7 @@ package org.apache.flex.html.staticControls.beads
>> >>
>> >>              listModel = value.getBeadByType(ISelectionModel) as
>> >> ISelectionModel;
>> >>              listModel.addEventListener("selectedIndexChanged",
>> >> selectionChangeHandler);
>> >> +            listModel.addEventListener("rollOverIndexChanged",
>> >> rollOverIndexChangeHandler);
>> >>
>> >>              _border = new Border();
>> >>              _border.model = new SingleLineBorderModel();
>> >> @@ -128,6 +130,23 @@ package
>>org.apache.flex.html.staticControls.beads
>> >>                         }
>> >>              lastSelectedIndex = listModel.selectedIndex;
>> >>                 }
>> >> +
>> >> +               private var lastRollOverIndex:int = -1;
>> >> +
>> >> +               private function
>> >> rollOverIndexChangeHandler(event:Event):void
>> >> +               {
>> >> +                       if (lastRollOverIndex != -1)
>> >> +                       {
>> >> +                               var ir:IItemRenderer =
>> >> dataGroup.getItemRendererForIndex(lastRollOverIndex) as
>>IItemRenderer;
>> >> +                ir.hovered = false;
>> >> +                       }
>> >> +                       if (IRollOverModel(listModel).rollOverIndex
>>!=
>> >>-1)
>> >> +                       {
>> >> +                   ir =
>> >>
>>
>>>>dataGroup.getItemRendererForIndex(IRollOverModel(listModel).rollOverInd
>>>>ex
>> >>);
>> >> +                   ir.hovered = true;
>> >> +                       }
>> >> +                       lastRollOverIndex =
>> >> IRollOverModel(listModel).rollOverIndex;
>> >> +               }
>> >>
>> >>                 private function createScrollBar():ScrollBar
>> >>                 {
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>
>>>>/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRende
>>>>re
>> >>rMouseController.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ItemRendererMouseController.as
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ItemRendererMouseController.as
>> >> index 87d9a58..fbc0b1c 100644
>> >> ---
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ItemRendererMouseController.as
>> >> +++
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ItemRendererMouseController.as
>> >> @@ -49,6 +49,7 @@ package
>> >> org.apache.flex.html.staticControls.beads.controllers
>> >>                         if (target)
>> >>                         {
>> >>                  target.hovered = true;
>> >> +                               target.dispatchEvent(new
>> >> Event("rollover"));
>> >>                         }
>> >>                 }
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>
>>>>/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingl
>>>>eS
>> >>electionMouseController.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ListSingleSelectionMouseController.as
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ListSingleSelectionMouseController.as
>> >> index dc69476..ea870ab 100644
>> >> ---
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ListSingleSelectionMouseController.as
>> >> +++
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controlle
>>>>rs
>> >>/ListSingleSelectionMouseController.as
>> >> @@ -21,11 +21,12 @@ package
>> >> org.apache.flex.html.staticControls.beads.controllers
>> >>         import org.apache.flex.core.IBeadController;
>> >>         import org.apache.flex.core.IItemRenderer;
>> >>         import org.apache.flex.core.IItemRendererParent;
>> >> +       import org.apache.flex.core.IRollOverModel;
>> >>         import org.apache.flex.core.ISelectionModel;
>> >>         import org.apache.flex.core.IStrand;
>> >> -       import org.apache.flex.html.staticControls.beads.IListView;
>> >>         import org.apache.flex.events.Event;
>> >>         import org.apache.flex.events.IEventDispatcher;
>> >> +       import org.apache.flex.html.staticControls.beads.IListView;
>> >>
>> >>
>> >>         public class ListSingleSelectionMouseController implements
>> >> IBeadController
>> >> @@ -47,6 +48,7 @@ package
>> >> org.apache.flex.html.staticControls.beads.controllers
>> >>                         listView = value.getBeadByType(IListView) as
>> >> IListView;
>> >>                         dataGroup = listView.dataGroup;
>> >>              dataGroup.addEventListener("selected", selectedHandler,
>> >>true);
>> >> +            dataGroup.addEventListener("rollover", rolloverHandler,
>> >>true);
>> >>                 }
>> >>
>> >>          private function selectedHandler(event:Event):void
>> >> @@ -54,6 +56,12 @@ package
>> >> org.apache.flex.html.staticControls.beads.controllers
>> >>              listModel.selectedIndex =
>> >>IItemRenderer(event.target).index;
>> >>              IEventDispatcher(listView.strand).dispatchEvent(new
>> >> Event("change"));
>> >>          }
>> >> +
>> >> +        private function rolloverHandler(event:Event):void
>> >> +        {
>> >> +            IRollOverModel(listModel).rollOverIndex =
>> >> IItemRenderer(event.target).index;
>> >> +            IEventDispatcher(listView.strand).dispatchEvent(new
>> >> Event("rollover"));
>> >> +        }
>> >>
>> >>         }
>> >>  }
>> >> \ No newline at end of file
>> >>
>> >>
>> >>
>> >>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>
>>>>/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelection
>>>>Mo
>> >>del.as
>> >>
>>----------------------------------------------------------------------
>> >> diff --git
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Ar
>>>>ra
>> >>ySelectionModel.as
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Ar
>>>>ra
>> >>ySelectionModel.as
>> >> index 3ca7f94..d1794be 100644
>> >> ---
>> >>
>>
>>>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Ar
>>>>ra
>> >>ySelectionModel.as
>> >> +++
>> >>
>>
>>>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Ar
>>>>ra
>> >>ySelectionModel.as
>> >> @@ -18,12 +18,13 @@
>> >>
>> >>
>>
>>>>///////////////////////////////////////////////////////////////////////
>>>>//
>> >>///////
>> >>  package org.apache.flex.html.staticControls.beads.models
>> >>  {
>> >> +       import org.apache.flex.core.IRollOverModel;
>> >>         import org.apache.flex.core.ISelectionModel;
>> >>         import org.apache.flex.core.IStrand;
>> >>         import org.apache.flex.events.Event;
>> >>         import org.apache.flex.events.EventDispatcher;
>> >>
>> >> -       public class ArraySelectionModel extends EventDispatcher
>> >> implements ISelectionModel
>> >> +       public class ArraySelectionModel extends EventDispatcher
>> >> implements ISelectionModel, IRollOverModel
>> >>         {
>> >>                 public function ArraySelectionModel()
>> >>                 {
>> >> @@ -49,6 +50,7 @@ package
>> >>org.apache.flex.html.staticControls.beads.models
>> >>                 }
>> >>
>> >>                 private var _selectedIndex:int = -1;
>> >> +               private var _rollOverIndex:int = -1;
>> >>
>> >>                 public function get selectedIndex():int
>> >>                 {
>> >> @@ -61,6 +63,16 @@ package
>> >>org.apache.flex.html.staticControls.beads.models
>> >>                         dispatchEvent(new
>> >>Event("selectedIndexChanged"));
>> >>                 }
>> >>
>> >> +               public function get rollOverIndex():int
>> >> +               {
>> >> +                       return _rollOverIndex;
>> >> +               }
>> >> +               public function set rollOverIndex(value:int):void
>> >> +               {
>> >> +                       _rollOverIndex = value;
>> >> +                       dispatchEvent(new
>> >>Event("rollOverIndexChanged"));
>> >> +               }
>> >> +
>> >>                 private var _selectedItem:Object;
>> >>
>> >>                 public function get selectedItem():Object
>> >>
>> >>
>>
>>


Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Tue, Nov 12, 2013 at 8:27 PM, Alex Harui <ah...@adobe.com> wrote:

> Hi Om,
>
> Looks good.  Some points to ponder:
>
> 1) Ideally, in a Pay-as-you-go philosophy, a rolloverIndex would not be in
> the ArraySelectionModel since, in theory, ArraySelectionModel should be
> reusable in mobile devices where there is no rollover.  I honestly don't
> know what the right answer is.  There are options like having a subclass
> of ArraySelectionModel that supports rollover.  There is the option of
> having rolloverIndex stored somewhere else.  Maybe rollover is a
> presentation concept, or maybe rollover should be entirely encapsulated in
> its own bead so it can be dropped out of mobile configs?  But I think the
> entire mousecontroller will be replaced by a touchcontroller in that case.
>  Another thing to consider is that, if a JS DG happens to wrap a
> third-party DG or I think there is one built into HTML5, and the JS
> version doesn't support rollover, how can we extract rollover from the AS
> version?
>

Yes, this was bothering me as well.  I couldnt think of a great place to
put this info.  Since it is not possible to have multiple models for a
component, having a subclass of ArraySelectionModel called
ArraySelectionAndRollOverModel would probably work.  But it will soon start
getting unwieldy if go this route.

The other option is to change ArraySelectionModel to ArrayInteractionsModel
and have an array of composed models, ex. SelectionModel, RollOverModel,
TouchModel, etc.  This way, we can string as many models as needed on a
pay-as-you-go basis.

What do you think?



>
> 2) There is an entirely alternate implementation of rollover where it is
> completely delegated to the renderer to determine based on mouse events
> what its display/rollover and selection state is.  Think of renderers that
> have buttons in them for deleting that row.  Clicking on the button may or
> may not change the selection, rollover the button may not cause rollover
> highlighting of the row, just the button.  I'm not saying this a preferred
> implementation, just that we want to make sure we make as many other beads
> reusable as possible in such an implementation, and not lock folks into
> one implementation or another.
>

So, each renderer has selection, rollover, etc. models?

Thanks,
Om


>
> Anyway, that last point is mainly food for thought.  From what I could
> tell from the diff this is a perfectly fine implementation of rollover,
> the only thing that stood out was adding rolloverindex to
> ArraySelectionModel.  Any thoughts on where else to hang it?  Of course,
> there is the argument that it is just one property slot, but I can promise
> you I've seen that philosophy cause slow but steady size and dependency
> creep.
>
> Thanks,
> -Alex
>
> On 11/12/13 6:09 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:
>
> >This is my first real contribution to the FlexJS project.  Alex/Peter can
> >you please review this and let me know if I am on the right track?
> >Once you approve, I will start looking at the JS version of the same
> >logic.
> >
> >Thanks,
> >Om
> >
> >
> >On Tue, Nov 12, 2013 at 6:07 PM, <bi...@apache.org> wrote:
> >
> >> Updated Branches:
> >>   refs/heads/develop 22f891823 -> 67c11796e
> >>
> >>
> >> Add functionality to AS version of DataGrid to highlight entire row when
> >> rolling over any column
> >> JS version to follow after discussion on dev@f.a.o
> >>
> >>
> >> Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
> >> Commit:
> http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/67c11796
> >> Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/67c11796
> >> Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/67c11796
> >>
> >> Branch: refs/heads/develop
> >> Commit: 67c11796e271bf7811e8afb12f556aa2386d2960
> >> Parents: 22f8918
> >> Author: Om <bi...@gmail.com>
> >> Authored: Tue Nov 12 18:05:48 2013 -0800
> >> Committer: Om <bi...@gmail.com>
> >> Committed: Tue Nov 12 18:06:26 2013 -0800
> >>
> >> ----------------------------------------------------------------------
> >>  .../src/org/apache/flex/core/IRollOverModel.as  | 28
> >>++++++++++++++++++++
> >>  .../org/apache/flex/html/staticControls/List.as | 12 ++++++++-
> >>  .../html/staticControls/beads/DataGridView.as   | 13 +++++++++
> >>  .../flex/html/staticControls/beads/ListView.as  | 19 +++++++++++++
> >>  .../controllers/ItemRendererMouseController.as  |  1 +
> >>  .../ListSingleSelectionMouseController.as       | 10 ++++++-
> >>  .../beads/models/ArraySelectionModel.as         | 14 +++++++++-
> >>  7 files changed, 94 insertions(+), 3 deletions(-)
> >> ----------------------------------------------------------------------
> >>
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/core/IRollOverModel.as
> >> ----------------------------------------------------------------------
> >> diff --git a/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> >> b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> >> new file mode 100644
> >> index 0000000..d38aa41
> >> --- /dev/null
> >> +++ b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
> >> @@ -0,0 +1,28 @@
> >>
> >>
> >>+////////////////////////////////////////////////////////////////////////
> >>////////
> >> +//
> >> +//  Licensed to the Apache Software Foundation (ASF) under one or more
> >> +//  contributor license agreements.  See the NOTICE file distributed
> >>with
> >> +//  this work for additional information regarding copyright ownership.
> >> +//  The ASF licenses this file to You under the Apache License, Version
> >> 2.0
> >> +//  (the "License"); you may not use this file except in compliance
> >>with
> >> +//  the License.  You may obtain a copy of the License at
> >> +//
> >> +//      http://www.apache.org/licenses/LICENSE-2.0
> >> +//
> >> +//  Unless required by applicable law or agreed to in writing, software
> >> +//  distributed under the License is distributed on an "AS IS" BASIS,
> >> +//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> >> implied.
> >> +//  See the License for the specific language governing permissions and
> >> +//  limitations under the License.
> >> +//
> >>
> >>
> >>+////////////////////////////////////////////////////////////////////////
> >>////////
> >> +package org.apache.flex.core
> >> +{
> >> +       import org.apache.flex.events.IEventDispatcher;
> >> +
> >> +       public interface IRollOverModel extends IEventDispatcher,
> >> IBeadModel
> >> +       {
> >> +               function get rollOverIndex():int;
> >> +               function set rollOverIndex(value:int):void;
> >> +       }
> >> +}
> >> \ No newline at end of file
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/List.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> >> b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> >> index 7cb3b6d..da1638e 100644
> >> --- a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> >> +++ b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
> >> @@ -18,8 +18,9 @@
> >>
> >>
> >>/////////////////////////////////////////////////////////////////////////
> >>///////
> >>  package org.apache.flex.html.staticControls
> >>  {
> >> +       import org.apache.flex.core.IRollOverModel;
> >>         import org.apache.flex.core.ISelectionModel;
> >> -    import org.apache.flex.core.UIBase;
> >> +       import org.apache.flex.core.UIBase;
> >>         import org.apache.flex.core.ValuesManager;
> >>         import
> >>
> >>org.apache.flex.html.staticControls.beads.IDataProviderItemRendererMapper
> >>;
> >>
> >> @@ -56,6 +57,15 @@ package org.apache.flex.html.staticControls
> >>                 {
> >>                         ISelectionModel(model).selectedIndex = value;
> >>                 }
> >> +
> >> +        public function get rollOverIndex():int
> >> +               {
> >> +                       return IRollOverModel(model).rollOverIndex;
> >> +               }
> >> +               public function set rollOverIndex(value:int):void
> >> +               {
> >> +                       IRollOverModel(model).rollOverIndex = value;
> >> +               }
> >>
> >>                 public function get selectedItem():Object
> >>                 {
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
> >>w.as
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
> >>w.as
> >> index 4088734..f2d3cc2 100644
> >> ---
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
> >>w.as
> >> +++
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
> >>w.as
> >> @@ -85,6 +85,7 @@ package org.apache.flex.html.staticControls.beads
> >>                                 columns.push(column);
> >>
> >>
> >> column.addEventListener('change',columnListChangeHandler);
> >> +
> >> column.addEventListener('rollover',columnListRollOverHandler);
> >>                         }
> >>
> >>
> >>
> >>IEventDispatcher(_strand).addEventListener("widthChanged",handleSizeChang
> >>e);
> >> @@ -139,5 +140,17 @@ package org.apache.flex.html.staticControls.beads
> >>
> >>                         IEventDispatcher(_strand).dispatchEvent(new
> >> Event('change'));
> >>                 }
> >> +
> >> +               private function
> >> columnListRollOverHandler(event:Event):void
> >> +               {
> >> +                       var list:List = event.target as List;
> >> +                       for(var i:int=0; i < columns.length; i++) {
> >> +                               if (list != columns[i]) {
> >> +                                       columns[i].rollOverIndex =
> >> list.rollOverIndex;
> >> +                               }
> >> +                       }
> >> +
> >> +                       IEventDispatcher(_strand).dispatchEvent(new
> >> Event('rollOver'));
> >> +               }
> >>         }
> >>  }
> >> \ No newline at end of file
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> >> index dbe9e36..b8c9ab9 100644
> >> ---
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> >> +++
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
> >> @@ -27,6 +27,7 @@ package org.apache.flex.html.staticControls.beads
> >>         import org.apache.flex.core.IItemRendererParent;
> >>         import org.apache.flex.core.ILayoutParent;
> >>         import org.apache.flex.core.IParent;
> >> +       import org.apache.flex.core.IRollOverModel;
> >>         import org.apache.flex.core.ISelectionModel;
> >>         import org.apache.flex.core.IStrand;
> >>         import org.apache.flex.core.Strand;
> >> @@ -96,6 +97,7 @@ package org.apache.flex.html.staticControls.beads
> >>
> >>              listModel = value.getBeadByType(ISelectionModel) as
> >> ISelectionModel;
> >>              listModel.addEventListener("selectedIndexChanged",
> >> selectionChangeHandler);
> >> +            listModel.addEventListener("rollOverIndexChanged",
> >> rollOverIndexChangeHandler);
> >>
> >>              _border = new Border();
> >>              _border.model = new SingleLineBorderModel();
> >> @@ -128,6 +130,23 @@ package org.apache.flex.html.staticControls.beads
> >>                         }
> >>              lastSelectedIndex = listModel.selectedIndex;
> >>                 }
> >> +
> >> +               private var lastRollOverIndex:int = -1;
> >> +
> >> +               private function
> >> rollOverIndexChangeHandler(event:Event):void
> >> +               {
> >> +                       if (lastRollOverIndex != -1)
> >> +                       {
> >> +                               var ir:IItemRenderer =
> >> dataGroup.getItemRendererForIndex(lastRollOverIndex) as IItemRenderer;
> >> +                ir.hovered = false;
> >> +                       }
> >> +                       if (IRollOverModel(listModel).rollOverIndex !=
> >>-1)
> >> +                       {
> >> +                   ir =
> >>
> >>dataGroup.getItemRendererForIndex(IRollOverModel(listModel).rollOverIndex
> >>);
> >> +                   ir.hovered = true;
> >> +                       }
> >> +                       lastRollOverIndex =
> >> IRollOverModel(listModel).rollOverIndex;
> >> +               }
> >>
> >>                 private function createScrollBar():ScrollBar
> >>                 {
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendere
> >>rMouseController.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ItemRendererMouseController.as
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ItemRendererMouseController.as
> >> index 87d9a58..fbc0b1c 100644
> >> ---
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ItemRendererMouseController.as
> >> +++
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ItemRendererMouseController.as
> >> @@ -49,6 +49,7 @@ package
> >> org.apache.flex.html.staticControls.beads.controllers
> >>                         if (target)
> >>                         {
> >>                  target.hovered = true;
> >> +                               target.dispatchEvent(new
> >> Event("rollover"));
> >>                         }
> >>                 }
> >>
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleS
> >>electionMouseController.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ListSingleSelectionMouseController.as
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ListSingleSelectionMouseController.as
> >> index dc69476..ea870ab 100644
> >> ---
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ListSingleSelectionMouseController.as
> >> +++
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
> >>/ListSingleSelectionMouseController.as
> >> @@ -21,11 +21,12 @@ package
> >> org.apache.flex.html.staticControls.beads.controllers
> >>         import org.apache.flex.core.IBeadController;
> >>         import org.apache.flex.core.IItemRenderer;
> >>         import org.apache.flex.core.IItemRendererParent;
> >> +       import org.apache.flex.core.IRollOverModel;
> >>         import org.apache.flex.core.ISelectionModel;
> >>         import org.apache.flex.core.IStrand;
> >> -       import org.apache.flex.html.staticControls.beads.IListView;
> >>         import org.apache.flex.events.Event;
> >>         import org.apache.flex.events.IEventDispatcher;
> >> +       import org.apache.flex.html.staticControls.beads.IListView;
> >>
> >>
> >>         public class ListSingleSelectionMouseController implements
> >> IBeadController
> >> @@ -47,6 +48,7 @@ package
> >> org.apache.flex.html.staticControls.beads.controllers
> >>                         listView = value.getBeadByType(IListView) as
> >> IListView;
> >>                         dataGroup = listView.dataGroup;
> >>              dataGroup.addEventListener("selected", selectedHandler,
> >>true);
> >> +            dataGroup.addEventListener("rollover", rolloverHandler,
> >>true);
> >>                 }
> >>
> >>          private function selectedHandler(event:Event):void
> >> @@ -54,6 +56,12 @@ package
> >> org.apache.flex.html.staticControls.beads.controllers
> >>              listModel.selectedIndex =
> >>IItemRenderer(event.target).index;
> >>              IEventDispatcher(listView.strand).dispatchEvent(new
> >> Event("change"));
> >>          }
> >> +
> >> +        private function rolloverHandler(event:Event):void
> >> +        {
> >> +            IRollOverModel(listModel).rollOverIndex =
> >> IItemRenderer(event.target).index;
> >> +            IEventDispatcher(listView.strand).dispatchEvent(new
> >> Event("rollover"));
> >> +        }
> >>
> >>         }
> >>  }
> >> \ No newline at end of file
> >>
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
> >>/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionMo
> >>del.as
> >> ----------------------------------------------------------------------
> >> diff --git
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
> >>ySelectionModel.as
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
> >>ySelectionModel.as
> >> index 3ca7f94..d1794be 100644
> >> ---
> >>
> >>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
> >>ySelectionModel.as
> >> +++
> >>
> >>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
> >>ySelectionModel.as
> >> @@ -18,12 +18,13 @@
> >>
> >>
> >>/////////////////////////////////////////////////////////////////////////
> >>///////
> >>  package org.apache.flex.html.staticControls.beads.models
> >>  {
> >> +       import org.apache.flex.core.IRollOverModel;
> >>         import org.apache.flex.core.ISelectionModel;
> >>         import org.apache.flex.core.IStrand;
> >>         import org.apache.flex.events.Event;
> >>         import org.apache.flex.events.EventDispatcher;
> >>
> >> -       public class ArraySelectionModel extends EventDispatcher
> >> implements ISelectionModel
> >> +       public class ArraySelectionModel extends EventDispatcher
> >> implements ISelectionModel, IRollOverModel
> >>         {
> >>                 public function ArraySelectionModel()
> >>                 {
> >> @@ -49,6 +50,7 @@ package
> >>org.apache.flex.html.staticControls.beads.models
> >>                 }
> >>
> >>                 private var _selectedIndex:int = -1;
> >> +               private var _rollOverIndex:int = -1;
> >>
> >>                 public function get selectedIndex():int
> >>                 {
> >> @@ -61,6 +63,16 @@ package
> >>org.apache.flex.html.staticControls.beads.models
> >>                         dispatchEvent(new
> >>Event("selectedIndexChanged"));
> >>                 }
> >>
> >> +               public function get rollOverIndex():int
> >> +               {
> >> +                       return _rollOverIndex;
> >> +               }
> >> +               public function set rollOverIndex(value:int):void
> >> +               {
> >> +                       _rollOverIndex = value;
> >> +                       dispatchEvent(new
> >>Event("rollOverIndexChanged"));
> >> +               }
> >> +
> >>                 private var _selectedItem:Object;
> >>
> >>                 public function get selectedItem():Object
> >>
> >>
>
>

Re: Code review please... Re: git commit: [flex-asjs] [refs/heads/develop] - Add functionality to AS version of DataGrid to highlight entire row when rolling over any column JS version to follow after discussion on dev@f.a.o

Posted by Alex Harui <ah...@adobe.com>.
Hi Om,

Looks good.  Some points to ponder:

1) Ideally, in a Pay-as-you-go philosophy, a rolloverIndex would not be in
the ArraySelectionModel since, in theory, ArraySelectionModel should be
reusable in mobile devices where there is no rollover.  I honestly don't
know what the right answer is.  There are options like having a subclass
of ArraySelectionModel that supports rollover.  There is the option of
having rolloverIndex stored somewhere else.  Maybe rollover is a
presentation concept, or maybe rollover should be entirely encapsulated in
its own bead so it can be dropped out of mobile configs?  But I think the
entire mousecontroller will be replaced by a touchcontroller in that case.
 Another thing to consider is that, if a JS DG happens to wrap a
third-party DG or I think there is one built into HTML5, and the JS
version doesn't support rollover, how can we extract rollover from the AS
version?

2) There is an entirely alternate implementation of rollover where it is
completely delegated to the renderer to determine based on mouse events
what its display/rollover and selection state is.  Think of renderers that
have buttons in them for deleting that row.  Clicking on the button may or
may not change the selection, rollover the button may not cause rollover
highlighting of the row, just the button.  I'm not saying this a preferred
implementation, just that we want to make sure we make as many other beads
reusable as possible in such an implementation, and not lock folks into
one implementation or another.

Anyway, that last point is mainly food for thought.  From what I could
tell from the diff this is a perfectly fine implementation of rollover,
the only thing that stood out was adding rolloverindex to
ArraySelectionModel.  Any thoughts on where else to hang it?  Of course,
there is the argument that it is just one property slot, but I can promise
you I've seen that philosophy cause slow but steady size and dependency
creep.

Thanks,
-Alex

On 11/12/13 6:09 PM, "OmPrakash Muppirala" <bi...@gmail.com> wrote:

>This is my first real contribution to the FlexJS project.  Alex/Peter can
>you please review this and let me know if I am on the right track?
>Once you approve, I will start looking at the JS version of the same
>logic.
>
>Thanks,
>Om
>
>
>On Tue, Nov 12, 2013 at 6:07 PM, <bi...@apache.org> wrote:
>
>> Updated Branches:
>>   refs/heads/develop 22f891823 -> 67c11796e
>>
>>
>> Add functionality to AS version of DataGrid to highlight entire row when
>> rolling over any column
>> JS version to follow after discussion on dev@f.a.o
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/67c11796
>> Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/67c11796
>> Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/67c11796
>>
>> Branch: refs/heads/develop
>> Commit: 67c11796e271bf7811e8afb12f556aa2386d2960
>> Parents: 22f8918
>> Author: Om <bi...@gmail.com>
>> Authored: Tue Nov 12 18:05:48 2013 -0800
>> Committer: Om <bi...@gmail.com>
>> Committed: Tue Nov 12 18:06:26 2013 -0800
>>
>> ----------------------------------------------------------------------
>>  .../src/org/apache/flex/core/IRollOverModel.as  | 28
>>++++++++++++++++++++
>>  .../org/apache/flex/html/staticControls/List.as | 12 ++++++++-
>>  .../html/staticControls/beads/DataGridView.as   | 13 +++++++++
>>  .../flex/html/staticControls/beads/ListView.as  | 19 +++++++++++++
>>  .../controllers/ItemRendererMouseController.as  |  1 +
>>  .../ListSingleSelectionMouseController.as       | 10 ++++++-
>>  .../beads/models/ArraySelectionModel.as         | 14 +++++++++-
>>  7 files changed, 94 insertions(+), 3 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/core/IRollOverModel.as
>> ----------------------------------------------------------------------
>> diff --git a/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> new file mode 100644
>> index 0000000..d38aa41
>> --- /dev/null
>> +++ b/frameworks/as/src/org/apache/flex/core/IRollOverModel.as
>> @@ -0,0 +1,28 @@
>>
>>
>>+////////////////////////////////////////////////////////////////////////
>>////////
>> +//
>> +//  Licensed to the Apache Software Foundation (ASF) under one or more
>> +//  contributor license agreements.  See the NOTICE file distributed
>>with
>> +//  this work for additional information regarding copyright ownership.
>> +//  The ASF licenses this file to You under the Apache License, Version
>> 2.0
>> +//  (the "License"); you may not use this file except in compliance
>>with
>> +//  the License.  You may obtain a copy of the License at
>> +//
>> +//      http://www.apache.org/licenses/LICENSE-2.0
>> +//
>> +//  Unless required by applicable law or agreed to in writing, software
>> +//  distributed under the License is distributed on an "AS IS" BASIS,
>> +//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> +//  See the License for the specific language governing permissions and
>> +//  limitations under the License.
>> +//
>>
>>
>>+////////////////////////////////////////////////////////////////////////
>>////////
>> +package org.apache.flex.core
>> +{
>> +       import org.apache.flex.events.IEventDispatcher;
>> +
>> +       public interface IRollOverModel extends IEventDispatcher,
>> IBeadModel
>> +       {
>> +               function get rollOverIndex():int;
>> +               function set rollOverIndex(value:int):void;
>> +       }
>> +}
>> \ No newline at end of file
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/List.as
>> ----------------------------------------------------------------------
>> diff --git
>>a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> index 7cb3b6d..da1638e 100644
>> --- a/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> +++ b/frameworks/as/src/org/apache/flex/html/staticControls/List.as
>> @@ -18,8 +18,9 @@
>>
>>
>>/////////////////////////////////////////////////////////////////////////
>>///////
>>  package org.apache.flex.html.staticControls
>>  {
>> +       import org.apache.flex.core.IRollOverModel;
>>         import org.apache.flex.core.ISelectionModel;
>> -    import org.apache.flex.core.UIBase;
>> +       import org.apache.flex.core.UIBase;
>>         import org.apache.flex.core.ValuesManager;
>>         import
>>
>>org.apache.flex.html.staticControls.beads.IDataProviderItemRendererMapper
>>;
>>
>> @@ -56,6 +57,15 @@ package org.apache.flex.html.staticControls
>>                 {
>>                         ISelectionModel(model).selectedIndex = value;
>>                 }
>> +
>> +        public function get rollOverIndex():int
>> +               {
>> +                       return IRollOverModel(model).rollOverIndex;
>> +               }
>> +               public function set rollOverIndex(value:int):void
>> +               {
>> +                       IRollOverModel(model).rollOverIndex = value;
>> +               }
>>
>>                 public function get selectedItem():Object
>>                 {
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/beads/DataGridView.as
>> ----------------------------------------------------------------------
>> diff --git
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
>>w.as
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
>>w.as
>> index 4088734..f2d3cc2 100644
>> ---
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
>>w.as
>> +++
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/DataGridVie
>>w.as
>> @@ -85,6 +85,7 @@ package org.apache.flex.html.staticControls.beads
>>                                 columns.push(column);
>>
>>
>> column.addEventListener('change',columnListChangeHandler);
>> +
>> column.addEventListener('rollover',columnListRollOverHandler);
>>                         }
>>
>>
>>
>>IEventDispatcher(_strand).addEventListener("widthChanged",handleSizeChang
>>e);
>> @@ -139,5 +140,17 @@ package org.apache.flex.html.staticControls.beads
>>
>>                         IEventDispatcher(_strand).dispatchEvent(new
>> Event('change'));
>>                 }
>> +
>> +               private function
>> columnListRollOverHandler(event:Event):void
>> +               {
>> +                       var list:List = event.target as List;
>> +                       for(var i:int=0; i < columns.length; i++) {
>> +                               if (list != columns[i]) {
>> +                                       columns[i].rollOverIndex =
>> list.rollOverIndex;
>> +                               }
>> +                       }
>> +
>> +                       IEventDispatcher(_strand).dispatchEvent(new
>> Event('rollOver'));
>> +               }
>>         }
>>  }
>> \ No newline at end of file
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>> ----------------------------------------------------------------------
>> diff --git
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>> index dbe9e36..b8c9ab9 100644
>> ---
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>> +++
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/ListView.as
>> @@ -27,6 +27,7 @@ package org.apache.flex.html.staticControls.beads
>>         import org.apache.flex.core.IItemRendererParent;
>>         import org.apache.flex.core.ILayoutParent;
>>         import org.apache.flex.core.IParent;
>> +       import org.apache.flex.core.IRollOverModel;
>>         import org.apache.flex.core.ISelectionModel;
>>         import org.apache.flex.core.IStrand;
>>         import org.apache.flex.core.Strand;
>> @@ -96,6 +97,7 @@ package org.apache.flex.html.staticControls.beads
>>
>>              listModel = value.getBeadByType(ISelectionModel) as
>> ISelectionModel;
>>              listModel.addEventListener("selectedIndexChanged",
>> selectionChangeHandler);
>> +            listModel.addEventListener("rollOverIndexChanged",
>> rollOverIndexChangeHandler);
>>
>>              _border = new Border();
>>              _border.model = new SingleLineBorderModel();
>> @@ -128,6 +130,23 @@ package org.apache.flex.html.staticControls.beads
>>                         }
>>              lastSelectedIndex = listModel.selectedIndex;
>>                 }
>> +
>> +               private var lastRollOverIndex:int = -1;
>> +
>> +               private function
>> rollOverIndexChangeHandler(event:Event):void
>> +               {
>> +                       if (lastRollOverIndex != -1)
>> +                       {
>> +                               var ir:IItemRenderer =
>> dataGroup.getItemRendererForIndex(lastRollOverIndex) as IItemRenderer;
>> +                ir.hovered = false;
>> +                       }
>> +                       if (IRollOverModel(listModel).rollOverIndex !=
>>-1)
>> +                       {
>> +                   ir =
>>
>>dataGroup.getItemRendererForIndex(IRollOverModel(listModel).rollOverIndex
>>);
>> +                   ir.hovered = true;
>> +                       }
>> +                       lastRollOverIndex =
>> IRollOverModel(listModel).rollOverIndex;
>> +               }
>>
>>                 private function createScrollBar():ScrollBar
>>                 {
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/beads/controllers/ItemRendere
>>rMouseController.as
>> ----------------------------------------------------------------------
>> diff --git
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ItemRendererMouseController.as
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ItemRendererMouseController.as
>> index 87d9a58..fbc0b1c 100644
>> ---
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ItemRendererMouseController.as
>> +++
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ItemRendererMouseController.as
>> @@ -49,6 +49,7 @@ package
>> org.apache.flex.html.staticControls.beads.controllers
>>                         if (target)
>>                         {
>>                  target.hovered = true;
>> +                               target.dispatchEvent(new
>> Event("rollover"));
>>                         }
>>                 }
>>
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/beads/controllers/ListSingleS
>>electionMouseController.as
>> ----------------------------------------------------------------------
>> diff --git
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ListSingleSelectionMouseController.as
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ListSingleSelectionMouseController.as
>> index dc69476..ea870ab 100644
>> ---
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ListSingleSelectionMouseController.as
>> +++
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/controllers
>>/ListSingleSelectionMouseController.as
>> @@ -21,11 +21,12 @@ package
>> org.apache.flex.html.staticControls.beads.controllers
>>         import org.apache.flex.core.IBeadController;
>>         import org.apache.flex.core.IItemRenderer;
>>         import org.apache.flex.core.IItemRendererParent;
>> +       import org.apache.flex.core.IRollOverModel;
>>         import org.apache.flex.core.ISelectionModel;
>>         import org.apache.flex.core.IStrand;
>> -       import org.apache.flex.html.staticControls.beads.IListView;
>>         import org.apache.flex.events.Event;
>>         import org.apache.flex.events.IEventDispatcher;
>> +       import org.apache.flex.html.staticControls.beads.IListView;
>>
>>
>>         public class ListSingleSelectionMouseController implements
>> IBeadController
>> @@ -47,6 +48,7 @@ package
>> org.apache.flex.html.staticControls.beads.controllers
>>                         listView = value.getBeadByType(IListView) as
>> IListView;
>>                         dataGroup = listView.dataGroup;
>>              dataGroup.addEventListener("selected", selectedHandler,
>>true);
>> +            dataGroup.addEventListener("rollover", rolloverHandler,
>>true);
>>                 }
>>
>>          private function selectedHandler(event:Event):void
>> @@ -54,6 +56,12 @@ package
>> org.apache.flex.html.staticControls.beads.controllers
>>              listModel.selectedIndex =
>>IItemRenderer(event.target).index;
>>              IEventDispatcher(listView.strand).dispatchEvent(new
>> Event("change"));
>>          }
>> +
>> +        private function rolloverHandler(event:Event):void
>> +        {
>> +            IRollOverModel(listModel).rollOverIndex =
>> IItemRenderer(event.target).index;
>> +            IEventDispatcher(listView.strand).dispatchEvent(new
>> Event("rollover"));
>> +        }
>>
>>         }
>>  }
>> \ No newline at end of file
>>
>>
>>
>>http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/67c11796/frameworks
>>/as/src/org/apache/flex/html/staticControls/beads/models/ArraySelectionMo
>>del.as
>> ----------------------------------------------------------------------
>> diff --git
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
>>ySelectionModel.as
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
>>ySelectionModel.as
>> index 3ca7f94..d1794be 100644
>> ---
>>
>>a/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
>>ySelectionModel.as
>> +++
>>
>>b/frameworks/as/src/org/apache/flex/html/staticControls/beads/models/Arra
>>ySelectionModel.as
>> @@ -18,12 +18,13 @@
>>
>>
>>/////////////////////////////////////////////////////////////////////////
>>///////
>>  package org.apache.flex.html.staticControls.beads.models
>>  {
>> +       import org.apache.flex.core.IRollOverModel;
>>         import org.apache.flex.core.ISelectionModel;
>>         import org.apache.flex.core.IStrand;
>>         import org.apache.flex.events.Event;
>>         import org.apache.flex.events.EventDispatcher;
>>
>> -       public class ArraySelectionModel extends EventDispatcher
>> implements ISelectionModel
>> +       public class ArraySelectionModel extends EventDispatcher
>> implements ISelectionModel, IRollOverModel
>>         {
>>                 public function ArraySelectionModel()
>>                 {
>> @@ -49,6 +50,7 @@ package
>>org.apache.flex.html.staticControls.beads.models
>>                 }
>>
>>                 private var _selectedIndex:int = -1;
>> +               private var _rollOverIndex:int = -1;
>>
>>                 public function get selectedIndex():int
>>                 {
>> @@ -61,6 +63,16 @@ package
>>org.apache.flex.html.staticControls.beads.models
>>                         dispatchEvent(new
>>Event("selectedIndexChanged"));
>>                 }
>>
>> +               public function get rollOverIndex():int
>> +               {
>> +                       return _rollOverIndex;
>> +               }
>> +               public function set rollOverIndex(value:int):void
>> +               {
>> +                       _rollOverIndex = value;
>> +                       dispatchEvent(new
>>Event("rollOverIndexChanged"));
>> +               }
>> +
>>                 private var _selectedItem:Object;
>>
>>                 public function get selectedItem():Object
>>
>>