Different controllers will have different behaviors.  Menu behavior is in fact 
one of the reasons I wanted to do beads and have replacable component logic.  
In your standard List, you "must" mouseDown and mouseUp on the same renderer to 
generate a Click that changes selection.  However, in many menus, you can 
mouseDown on the MenuBar and without releasing the mouse button, drag down the 
menu to an item and release (MouseUp) and that item will be selected from the 
menu.  Outlook's menubar on my Mac seems to work this way.  So, some people 
will want mouseUp to change selection, others will want click to change 
selection.  Royale should offer choices.

But changing selection is a higher-level event.  The lowest level events are 
mouseDown/mouseUp/click.  In between is the "ITEM_CLICK" event and in theor, 
ITEM_MOUSEDOWN and ITEM_MOUSEUP.  It is up to the controllers to interpret a 
lower-level event and then dispatch a higher-level event.   It seems 
"illogical" to me that an ITEM_CLICK would be dispatched from a mouseUp 
handler.  ITEM_CLICK (and ITEM_MOUSEUP if there is such a things) is meant to 
indicate that an event happened on the renderer that should be interpreted by 
the controller.  Some parts of a renderer may not dispatch ITEM_CLICK (for 
example, in some Trees, clcking on expand/collapse icons don't change selection.

Then the MenuController and ListSelectionController should be interpreting 
ITEM_CLICK and ITEM_MOUSEUP and deciding what to do.  So maybe the answer is 
that the renderer's mouseUp controller dispatches ITEM_MOUSEUP, and Menu 
changes selection on ITEM_CLICK or ITEM_MOUSEUP.

Separately, whether setTimeout or requestAnimationFrame, the more deferred work 
and asynchronicity we put in the framework, the more trouble we will give our 
users IMO.  Starting such a trend has the danger of making application code use 
more and more callLater/setTimeout/requestAnimationFrame calls as well.  We 
should not defer work unless we have to, and I don't think we have to in this 
case.  Making things rely on timing creates the potential for timing issues.

My 2 cents,
-Alex

On 9/13/18, 1:50 PM, "Harbs" <[email protected]> wrote:

    That article was not what I was referring to. Like I said I couldn’t find 
it. I’m working from memory.
    
    > I'm still surprised the fix isn't to not dispatch ITEM_CLICK on MouseUp.  
  UP doesn't guarantee CLICK, IMO.  You could mouse down in one renderer and 
mouse up in another renderer.  In some interaction models that does change 
selection, but I don't think CLICK is the right semantic.
    
    I don’t understand you. Are you saying that mousing down on one renderer 
and mousing up on another should fire an itemClick? That seems wrong to me. An 
itemClick should be both mouse down and mouse up on the same item. Click does 
that very nicely.
    
    > Further, I would like to discourage use of setTimeout in the framework.  
The framework has little control over the behavior of application event 
handlers.
    
    I would generally agree with you. But in this case, we’re handling the 
dismissal of a menu. I don’t see the harm here. If you have another proposal, 
I’m open to hear…
    
    We can also use requestAnimationFrame() to delay the execution until after 
click. That will be guaranteed to be before the next draw of the screen, so it 
might be better than setTimeout() which will probably be later.
    
    Thanks,
    Harbs
    
    > On Sep 13, 2018, at 11:20 PM, Alex Harui <[email protected]> wrote:
    > 
    > I didn't see anywhere in the article where it guaranteed that both up and 
click are sequential in the queue.  Sharing the heap isn't the issue here.
    > 
    > I'm still surprised the fix isn't to not dispatch ITEM_CLICK on MouseUp.  
  UP doesn't guarantee CLICK, IMO.  You could mouse down in one renderer and 
mouse up in another renderer.  In some interaction models that does change 
selection, but I don't think CLICK is the right semantic.
    > 
    > Further, I would like to discourage use of setTimeout in the framework.  
The framework has little control over the behavior of application event 
handlers.  With many browsers having immediate screen updates (as opposed to 
Flash's deferred rendering), postponing work with setTimeout can result in 
unexpected behavior, and can result in callLater "wars" where more and more 
code has to keep deferring work with setTimeout because some lower layer also 
used setTimeout.  Deferring work should be the option of the application 
developer as much as possible.
    > 
    > My 2 cents,
    > -Alex
    > 
    > On 9/13/18, 11:36 AM, "Harbs" <[email protected]> wrote:
    > 
    >    I once read that the way setTimeout works is that it’s added to the 
next event loop. That would mean that both mouseup and click would be executed 
in the current event loop and the setTimout would be delayed to the next one 
(i..e between 4ms and 10ms later).
    > 
    >    I can’t find the article at the moment, but my understanding is that 
this is a safe way to guarantee later execution in all browsers.
    > 
    >    The MDN doc has a decent explanation.[1] Both mouseup and click would 
be on the same heap.
    > 
    >    Harbs
    > 
    >    
[1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FWeb%2FJavaScript%2FEventLoop&amp;data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&amp;sdata=P0peR5eRbvHOFN1DgfG7q2Lho4cSbH47z3l4m77Vx6s%3D&amp;reserved=0
    >> On Sep 13, 2018, at 6:54 PM, Alex Harui <[email protected]> wrote:
    >> 
    >> Is setTimeout guaranteed to run after the CLICK event?
    >> 
    >> On 9/13/18, 6:14 AM, "[email protected] <mailto:[email protected]>" 
<[email protected] <mailto:[email protected]>> wrote:
    >> 
    >>   This is an automated email from the ASF dual-hosted git repository.
    >> 
    >>   harbs pushed a commit to branch develop
    >>   in repository 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&amp;data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&amp;sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&amp;reserved=0
 
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&amp;data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&amp;sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&amp;reserved=0>
    >> 
    >> 
    >>   The following commit(s) were added to refs/heads/develop by this push:
    >>        new e1ec25e  I think this fixes Menu
    >>   e1ec25e is described below
    >> 
    >>   commit e1ec25e0d8b1e59bfbb1e3d0cf856fe9dfb4dc5e
    >>   Author: Harbs <[email protected] <mailto:[email protected]>>
    >>   AuthorDate: Thu Sep 13 16:14:03 2018 +0300
    >> 
    >>       I think this fixes Menu
    >>   ---
    >>    .../projects/Basic/src/main/resources/defaults.css |  1 +
    >>    .../controllers/ItemRendererMouseController.as     |  1 -
    >>    .../controllers/MenuSelectionMouseController.as    | 33 
++++++++++++++--------
    >>    3 files changed, 23 insertions(+), 12 deletions(-)
    >> 
    >>   diff --git a/frameworks/projects/Basic/src/main/resources/defaults.css 
b/frameworks/projects/Basic/src/main/resources/defaults.css
    >>   index f88da20..15cb4e6 100644
    >>   --- a/frameworks/projects/Basic/src/main/resources/defaults.css
    >>   +++ b/frameworks/projects/Basic/src/main/resources/defaults.css
    >>   @@ -545,6 +545,7 @@ Panel
    >>          IBeadModel: 
ClassReference("org.apache.royale.html.beads.models.PanelModel");
    >>          IBeadView: 
ClassReference("org.apache.royale.html.beads.PanelView");
    >>          IPanelLayout: 
ClassReference("org.apache.royale.html.beads.layouts.VerticalFlexLayout");
    >>   +      IPanelContentArea: 
ClassReference("org.apache.royale.html.Container");
    >>          
    >>          background-color: #FFFFFF;
    >>          border: 1px solid #333333
    >>   diff --git 
a/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/ItemRendererMouseController.as
 
b/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/ItemRendererMouseController.as
    >>   index f408a71..6073264 100644
    >>   --- 
a/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/ItemRendererMouseController.as
    >>   +++ 
b/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/ItemRendererMouseController.as
    >>   @@ -92,7 +92,6 @@ COMPILE::JS {
    >>                                  goog.events.listen(element, 
goog.events.EventType.MOUSEOUT, this.handleMouseOut);
    >>                                  goog.events.listen(element, 
goog.events.EventType.MOUSEDOWN, this.handleMouseDown);
    >>                                  goog.events.listen(element, 
goog.events.EventType.CLICK, this.handleMouseUp);
    >>   -                goog.events.listen(element, 
goog.events.EventType.MOUSEUP, this.handleMouseUp);
    >>                          }
    >>                  }
    >>                  
    >>   diff --git 
a/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/MenuSelectionMouseController.as
 
b/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/MenuSelectionMouseController.as
    >>   index 5f986bb..6a09a52 100644
    >>   --- 
a/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/MenuSelectionMouseController.as
    >>   +++ 
b/frameworks/projects/Basic/src/main/royale/org/apache/royale/html/beads/controllers/MenuSelectionMouseController.as
    >>   @@ -127,6 +127,8 @@ package org.apache.royale.html.beads.controllers
    >>                   *  @playerversion Flash 10.2
    >>                   *  @playerversion AIR 2.6
    >>                   *  @productversion Royale 0.9
    >>   +         *  @royaleignorecoercion org.apache.royale.core.UIBase
    >>   +         *  @royaleignorecoercion org.apache.royale.core.IUIBase
    >>                   */
    >>                  protected function hideOpenMenus():void
    >>                  {
    >>   @@ -137,8 +139,9 @@ package org.apache.royale.html.beads.controllers
    >>                                  if (menu.parent != null) {
    >>                                          var 
controller:MenuSelectionMouseController = 
menu.getBeadByType(MenuSelectionMouseController) as 
MenuSelectionMouseController;
    >>                                          
controller.removeClickOutHandler(menu);
    >>   -                    var host:IPopUpHost = 
UIUtils.findPopUpHost(_strand as IUIBase);
    >>   -                                      
host.popUpParent.removeElement(menu);
    >>   +                    var host:IPopUpHost = UIUtils.findPopUpHost(menu 
as IUIBase);
    >>   +                                      if(host)
    >>   +                                              
host.popUpParent.removeElement(menu);
    >>                                  }
    >>                          }
    >>                          MenuModel.clearMenuList();
    >>   @@ -163,6 +166,8 @@ package org.apache.royale.html.beads.controllers
    >>                   *  @playerversion Flash 10.2
    >>                   *  @playerversion AIR 2.6
    >>                   *  @productversion Royale 0.9
    >>   +         *  @royaleignorecoercion org.apache.royale.core.IUIBase
    >>   +         *  @royaleignorecoercion 
org.apache.royale.events.IEventDispatcher
    >>                   */
    >>                  public function removeClickOutHandler(menu:Object):void
    >>                  {
    >>   @@ -191,23 +196,29 @@ package org.apache.royale.html.beads.controllers
    >> 
    >>                  /**
    >>             * @royaleignorecoercion HTMLElement
    >>   +               * @royaleignorecoercion org.apache.royale.core.IUIBase
    >>                   * @private
    >>                   */
    >>                  COMPILE::JS
    >>                  protected function 
hideMenu_internal(event:BrowserEvent):void
    >>                  {                       
    >>                var menu:IMenu = _strand as IMenu;
    >>   +                      var menuElem:HTMLElement = (_strand as 
IUIBase).element as HTMLElement;
    >>   +                      var menuBarElement:HTMLElement;
    >>                if (menu.parentMenuBar)
    >>                {
    >>   -                var menuBarElement:HTMLElement = (menu.parentMenuBar 
as IUIBase).element as HTMLElement;
    >>   -                var target:HTMLElement = event.target as HTMLElement;
    >>   -                while (target != null)
    >>   -                {
    >>   -                    if (target == menuBarElement) return;
    >>   -                    target = target.parentNode as HTMLElement;
    >>   -                }
    >>   -            }
    >>   -                      hideOpenMenus();
    >>   +                menuBarElement = (menu.parentMenuBar as 
IUIBase).element as HTMLElement;
    >>   +                      }
    >>   +                      var target:HTMLElement = event.target as 
HTMLElement;
    >>   +                      while (target != null)
    >>   +                      {
    >>   +                              var comp:IUIBase = 
target["royale_wrapper"];
    >>   +                              if(comp && (comp is IMenu || comp == 
menu.parentMenuBar) ) return;
    >>   +                              // if (target == menuElem || 
(menuBarElement && target == menuBarElement) ) return;
    >>   +                              target = target.parentNode as 
HTMLElement;
    >>   +                      }
    >>   +            
    >>   +                      setTimeout(hideOpenMenus);
    >>                  }
    >>          }
    >>    }
    >>   \ No newline at end of file
    > 
    > 
    > 
    
    

Reply via email to