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" <bigosma...@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, <bigosma...@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 <bigosma...@gmail.com> >> Authored: Tue Nov 12 18:05:48 2013 -0800 >> Committer: Om <bigosma...@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 >> >>
