You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flex.apache.org by ah...@apache.org on 2017/06/06 18:57:58 UTC

[2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

fix event types for valuechange.  Otherwise Binding would break.  Events are global and should be compatible for any given event type


Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/4df71784
Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/4df71784
Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/4df71784

Branch: refs/heads/release0.8.0
Commit: 4df717847eaeb12d6b14af833853852eb3cc13ce
Parents: b17c2bd
Author: Alex Harui <ah...@apache.org>
Authored: Tue Jun 6 11:58:03 2017 -0700
Committer: Alex Harui <ah...@apache.org>
Committed: Tue Jun 6 11:58:03 2017 -0700

----------------------------------------------------------------------
 .../flex/html/beads/NumericStepperView.as       |  5 ++--
 .../beads/controllers/SliderMouseController.as  | 27 +++++++++++++++-----
 .../beads/controllers/SpinnerMouseController.as |  9 +++++--
 .../apache/flex/html/beads/models/RangeModel.as |  4 ++-
 .../html/beads/models/RangeModelExtended.as     |  9 +++++--
 .../html/beads/models/ValueToggleButtonModel.as |  5 +++-
 6 files changed, 44 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/NumericStepperView.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/NumericStepperView.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/NumericStepperView.as
index 15e2321..5ad42dc 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/NumericStepperView.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/NumericStepperView.as
@@ -29,6 +29,7 @@ package org.apache.flex.html.beads
     import org.apache.flex.core.IUIBase;
     import org.apache.flex.core.UIBase;
 	import org.apache.flex.events.Event;
+	import org.apache.flex.events.ValueChangeEvent
 	import org.apache.flex.events.IEventDispatcher;
     import org.apache.flex.html.Label;
 	import org.apache.flex.html.Spinner;
@@ -143,11 +144,11 @@ package org.apache.flex.html.beads
 		/**
 		 * @private
 		 */
-		private function spinnerValueChanged(event:Event) : void
+		private function spinnerValueChanged(event:ValueChangeEvent) : void
 		{
 			input.text = String(spinner.value);
 			
-			var newEvent:Event = new Event(event.type,event.bubbles);
+			var newEvent:ValueChangeEvent = new ValueChangeEvent(event.type, false, false, event.oldValue, event.newValue);
 			IEventDispatcher(_strand).dispatchEvent(newEvent);
 		}
 		

http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SliderMouseController.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SliderMouseController.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SliderMouseController.as
index 91589b5..9663aff 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SliderMouseController.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SliderMouseController.as
@@ -26,6 +26,7 @@ package org.apache.flex.html.beads.controllers
 	import org.apache.flex.events.Event;
 	import org.apache.flex.events.IEventDispatcher;
 	import org.apache.flex.events.MouseEvent;
+	import org.apache.flex.events.ValueChangeEvent;
 	import org.apache.flex.geom.Point;
 	import org.apache.flex.html.beads.ISliderView;
 	import org.apache.flex.html.beads.SliderTrackView;
@@ -68,7 +69,9 @@ package org.apache.flex.html.beads.controllers
 		private var rangeModel:IRangeModel;
 		
 		private var _strand:IStrand;
-		
+
+		private var oldValue:Number;
+				
 		/**
 		 *  @copy org.apache.flex.core.IBead#strand
 		 *  
@@ -126,6 +129,7 @@ package org.apache.flex.html.beads.controllers
 			
 			origin = new Point(event.screenX, event.screenY);
 			thumb = new Point(sliderView.thumb.x,sliderView.thumb.y);
+			oldValue = rangeModel.value;
 		}
 		
 		/**
@@ -137,7 +141,8 @@ package org.apache.flex.html.beads.controllers
 			UIBase(_strand).topMostEventDispatcher.removeEventListener(MouseEvent.MOUSE_MOVE, thumbMoveHandler);
 			UIBase(_strand).topMostEventDispatcher.removeEventListener(MouseEvent.MOUSE_UP, thumbUpHandler);
 			
-			IEventDispatcher(_strand).dispatchEvent(new Event("valueChange"));
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, oldValue, rangeModel.value);
+			IEventDispatcher(_strand).dispatchEvent(vce);
 		}
 		
         COMPILE::SWF
@@ -160,9 +165,10 @@ package org.apache.flex.html.beads.controllers
 			var p:Number = newX/sliderView.track.width;
 			var n:Number = p*(rangeModel.maximum - rangeModel.minimum) + rangeModel.minimum;
 		
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, rangeModel.value, n);
 			rangeModel.value = n;
 			
-			IEventDispatcher(_strand).dispatchEvent(new Event("valueChange"));
+			IEventDispatcher(_strand).dispatchEvent(vce);
 		}
 		
 		/**
@@ -179,9 +185,10 @@ package org.apache.flex.html.beads.controllers
 			var p:Number = xloc/sliderView.track.width;
 			var n:Number = p*(rangeModel.maximum - rangeModel.minimum) + rangeModel.minimum;
 			
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, rangeModel.value, n);
 			rangeModel.value = n;
 			
-			IEventDispatcher(_strand).dispatchEvent(new Event("valueChange"));
+			IEventDispatcher(_strand).dispatchEvent(vce);
 		}
         
         /**
@@ -195,6 +202,7 @@ package org.apache.flex.html.beads.controllers
             var n:Number = p * (host.maximum - host.minimum) +
                 host.minimum;
             
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, host.value, n);
             host.value = n;
             
             origin = parseInt(thumb.element.style.left, 10);
@@ -202,7 +210,7 @@ package org.apache.flex.html.beads.controllers
             
             calcValFromMousePosition(event, true);
             
-            host.dispatchEvent(new org.apache.flex.events.Event('valueChange'));
+            host.dispatchEvent(vce);
         }
         
         
@@ -219,6 +227,7 @@ package org.apache.flex.html.beads.controllers
             
             origin = event.clientX;
             position = parseInt(thumb.element.style.left, 10);
+            oldValue = rangeModel.value;
         }
         
         COMPILE::JS
@@ -238,8 +247,9 @@ package org.apache.flex.html.beads.controllers
                 handleThumbMove, false, this);
             
             calcValFromMousePosition(event, false);
+            var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, oldValue, rangeModel.value);
             
-            host.dispatchEvent(new org.apache.flex.events.Event('valueChange'));
+            host.dispatchEvent(vce);
         }
         
         
@@ -249,9 +259,12 @@ package org.apache.flex.html.beads.controllers
         private function handleThumbMove(event:BrowserEvent):void
         {
             var host:Slider = _strand as Slider;
+            var lastValue:Number = rangeModel.value;
             calcValFromMousePosition(event, false);
             
-            host.dispatchEvent(new org.apache.flex.events.Event('valueChange'));
+            var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, lastValue, rangeModel.value);
+            
+            host.dispatchEvent(vce);
         }
         
         

http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SpinnerMouseController.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SpinnerMouseController.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SpinnerMouseController.as
index 2a898d8..b228980 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SpinnerMouseController.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/controllers/SpinnerMouseController.as
@@ -25,6 +25,7 @@ package org.apache.flex.html.beads.controllers
 	import org.apache.flex.events.Event;
 	import org.apache.flex.events.IEventDispatcher;
 	import org.apache.flex.events.MouseEvent;
+	import org.apache.flex.events.ValueChangeEvent;
 	import org.apache.flex.html.TextButton;
 	import org.apache.flex.html.beads.ISpinnerView;
     COMPILE::JS
@@ -108,8 +109,10 @@ package org.apache.flex.html.beads.controllers
 		 */
 		private function decrementClickHandler( event:org.apache.flex.events.MouseEvent ) : void
 		{
+			var oldValue:Number = rangeModel.value;
 			rangeModel.value = Math.max(rangeModel.minimum, rangeModel.value - rangeModel.stepSize);
-			IEventDispatcher(_strand).dispatchEvent(new Event("valueChange"));
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, oldValue, rangeModel.value);
+			IEventDispatcher(_strand).dispatchEvent(vce);
 		}
 
 		/**
@@ -117,8 +120,10 @@ package org.apache.flex.html.beads.controllers
 		 */
 		private function incrementClickHandler( event:org.apache.flex.events.MouseEvent ) : void
 		{
+			var oldValue:Number = rangeModel.value;
 			rangeModel.value = Math.min(rangeModel.maximum, rangeModel.value + rangeModel.stepSize);
-			IEventDispatcher(_strand).dispatchEvent(new Event("valueChange"));
+			var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, oldValue, rangeModel.value);
+			IEventDispatcher(_strand).dispatchEvent(vce);
 		}
 	}
 }

http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModel.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModel.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModel.as
index c06d3fe..ab86d03 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModel.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModel.as
@@ -23,6 +23,7 @@ package org.apache.flex.html.beads.models
 	import org.apache.flex.core.IRangeModel;
 	import org.apache.flex.events.Event;
 	import org.apache.flex.events.EventDispatcher;
+	import org.apache.flex.events.ValueChangeEvent;
 			
 	/**
 	 *  The RangeModel class bead defines a set of for a numeric range of values
@@ -194,8 +195,9 @@ package org.apache.flex.html.beads.models
 				// what is coming in.
 				newValue = Math.max(minimum, newValue);
 				newValue = Math.min(maximum, newValue);
+				var oldValue:Number = _value;
 				_value = snap(newValue);
-				dispatchEvent(new Event("valueChange"));
+				dispatchEvent(new ValueChangeEvent("valueChange", false, false, oldValue, _value));
 			}
 		}
 		

http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModelExtended.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModelExtended.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModelExtended.as
index ebbc4f3..fad333e 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModelExtended.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/RangeModelExtended.as
@@ -19,6 +19,7 @@
 package org.apache.flex.html.beads.models
 {
 	import org.apache.flex.events.Event;
+	import org.apache.flex.events.ValueChangeEvent;
 	import org.apache.flex.html.beads.models.RangeModel;
 
 	/**
@@ -56,8 +57,12 @@ package org.apache.flex.html.beads.models
 		}
 		override public function set value(newValue:Number):void
 		{
-			_value = newValue;
-			dispatchEvent(new Event("valueChange"));
+			if (_value != newValue)
+			{
+				var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, _value, newValue);
+				_value = newValue;
+				dispatchEvent(vce);
+			}
 		}
 
 		private var _labelFunction:Function;

http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/4df71784/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/ValueToggleButtonModel.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/ValueToggleButtonModel.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/ValueToggleButtonModel.as
index 27b0a95..e8fbc76 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/ValueToggleButtonModel.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/beads/models/ValueToggleButtonModel.as
@@ -21,6 +21,8 @@ package org.apache.flex.html.beads.models
 	
 	import org.apache.flex.core.IValueToggleButtonModel;
 	import org.apache.flex.events.Event;
+	import org.apache.flex.events.ValueChangeEvent;
+	
 
 	/**
 	 *  The ValueToggleButtonModel class bead extends the 
@@ -67,8 +69,9 @@ package org.apache.flex.html.beads.models
 		{
 			if( newValue != _value )
 			{
+				var vce:ValueChangeEvent = new ValueChangeEvent("valueChange", false, false, _value, newValue);
 				_value = newValue;
-				dispatchEvent(new Event("valueChange"));
+				dispatchEvent(vce);
 			}
 		}
 		


Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Peter Ent <pe...@adobe.com.INVALID>.
I don't think PAYG is always obvious and I continue to struggle with the
question. Some examples:

a. Anything that can be expressed as a style probably does not need a
bead, unless the style is not native to the platform. Border and
backgroundColor are good examples. These are built into HTML, but not the
Flash Player. So the border and background styles are implemented as beads
but they are only available on the Flash Player platform. Width, height,
x, y, etc. are also styles in HTML but properties in Flash, so they don't
need beads either. Plus they are pretty fundamental to UI systems.

b. Completeness also gives me pangs. You would think that a slider's basic
properties of a minimum value, maximum value, and a current value would
not need to be questioned. And that's true, as properties they are
necessary to define a slider. But should a slider emit an event whenever
its min, max, or value changes? Many apps will just go get the current
value when they need it. The slider's mouse controller observes the
dynamic changes in the thumb position and emits an event for each movement
- but not everyone needs that so such a controller could be optional, but
without a controller, you can't actually change the thumb with the mouse
so you need the controller regardless, but do you need to emit the event
when the thumb is being moved?

c. When you do have to have something, like a slider mouse controller, to
make the component work at its basic level, do you make an option as to
whether or not to emit the event or do you have one controller that emits
events as the thumb moves and one controller that does not emit the event?
Is one controller a subclass of the other or a duplicate version?

d. Its often difficult to tell if something should be present just because
it always has been present (in Flex). When you have a set of properties
and events, we need to ask if those are essential to making the component
work. Or rather, what is the component's default behavior. For a text
input component, the default because is accepting keyboard input, without
filtering. Making it a secure input with a mask, is extra work/behavior
and thus becomes a bead. Beads enhance and augment the default behavior as
a rule of thumb.

PAYG for me, raises lots of questions about programming practices and
patterns. Do you make it easier on developers by packaging common
properties/events/options even if some are not used that often, or do you
break them down into many versions?

One way to handle this is by looking at the Express project. There, the
most commonly used patterns are put together. For instance, data binding
is in everything. Or you might have the slider always emit every event it
can. 

The Basic project should be the bare-bones components with lots of options
as beads. Then package them into common use cases and put that into
another project, like Express, to make it more palatable.

I would argue (just a little) that more complex components, like DataGrid,
and perhaps even NumericStepper, should be in a project that lives between
Basic and Express. This would leave Basic being very, well, basic (label,
button, text input, group and container for instance).

I hope that helps a little.

‹peter

On 6/7/17, 1:27 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> I agree that there could be a even smaller bead that doesn't track
>>changes
>> as you move the thumb.  Something to do in the next release, IMO.
>
>Could be or must be? Again just trying to understand why PAYG wasn¹t
>followed in this case.
>
>Was this just an accidental design omission that no body picked up on it
>OR is it sometimes OK to have optional feature code in beads?
>
>Thanks,
>Justin


Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,

> I don't think Justin's questions are about the value of PAYG; they rather
> indicate a genuine attempt at understanding the concept.

That’s correct.

Thanks,
Justin

Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by OmPrakash Muppirala <bi...@gmail.com>.
On Jun 6, 2017 10:42 PM, "Alex Harui" <ah...@adobe.com.invalid> wrote:

On 6/6/17, 10:27 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> I agree that there could be a even smaller bead that doesn't track
>>changes
>> as you move the thumb.  Something to do in the next release, IMO.
>
>Could be or must be? Again just trying to understand why PAYG wasn’t
>followed in this case.

I haven't really thought about it.  I'm still just trying to get a release
out.  User expectations are a factor.  Most folks won't need to remove the
PasswordInputBead.  Do most folks want to track the thumb?
>
>Was this just an accidental design omission that no body picked up on it
>OR is it sometimes OK to have optional feature code in beads?

Probably one of just many bugs we haven't found or fixed yet.  It is
unfortunate we spend all of this energy trying to convince you of the
value of PAYG instead of finding and fixing these bugs.


Alex,

I don't think Justin's questions are about the value of PAYG; they rather
indicate a genuine attempt at understanding the concept.

I suggest you reset your understanding of his intent and re-read his
emails.  Maybe you will see his line of questioning in a more positive
light.

Thanks,
Om






-Alex

Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Alex Harui <ah...@adobe.com.INVALID>.
On 6/6/17, 10:27 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> I agree that there could be a even smaller bead that doesn't track
>>changes
>> as you move the thumb.  Something to do in the next release, IMO.
>
>Could be or must be? Again just trying to understand why PAYG wasn’t
>followed in this case.

I haven't really thought about it.  I'm still just trying to get a release
out.  User expectations are a factor.  Most folks won't need to remove the
PasswordInputBead.  Do most folks want to track the thumb?
>
>Was this just an accidental design omission that no body picked up on it
>OR is it sometimes OK to have optional feature code in beads?

Probably one of just many bugs we haven't found or fixed yet.  It is
unfortunate we spend all of this energy trying to convince you of the
value of PAYG instead of finding and fixing these bugs.

-Alex




Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,

> I agree that there could be a even smaller bead that doesn't track changes
> as you move the thumb.  Something to do in the next release, IMO.

Could be or must be? Again just trying to understand why PAYG wasn’t followed in this case.

Was this just an accidental design omission that no body picked up on it OR is it sometimes OK to have optional feature code in beads?

Thanks,
Justin

Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Alex Harui <ah...@adobe.com.INVALID>.
Actually, I was just trying to eliminate an exception so we can ship a
release.

I agree that there could be a even smaller bead that doesn't track changes
as you move the thumb.  Something to do in the next release, IMO.

But I also saw that I hadn't quite implemented the change correctly, so
I've just pushed a better version.

Thanks,
-Alex

On 6/6/17, 5:14 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>I see you've added code to handle binding / values changes to
>RangeModel.as and RangeModelExtended.as.
>
>Is there any reason the value change code is not in another classes
>called for instance ValueChangeRangeModel? It seem to me that this model
>it trying to do more than one thing i.e. value changes and min/max/step
>changes.
>
>As I user of this I may not be interested minor max changes and only
>interested in value changes so shouldn't that be an optional feature?
>
>I’m just trying to understand why it’s seemingly OK to not follow PAYG
>here.
>
>Thanks,
>Justin


Re: [2/2] git commit: [flex-asjs] [refs/heads/release0.8.0] - fix event types for valuechange. Otherwise Binding would break. Events are global and should be compatible for any given event type

Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,

I see you've added code to handle binding / values changes to RangeModel.as and RangeModelExtended.as. 

Is there any reason the value change code is not in another classes called for instance ValueChangeRangeModel? It seem to me that this model it trying to do more than one thing i.e. value changes and min/max/step changes.

As I user of this I may not be interested minor max changes and only interested in value changes so shouldn't that be an optional feature?

I’m just trying to understand why it’s seemingly OK to not follow PAYG here.

Thanks,
Justin