OK. It sounds like you have a good plan. I’m fine with dispatching itemMouseDown, itemMouseUp and itemClick. That makes sense and should solve the problem.
Do you want me to work on implementing that? > On Sep 14, 2018, at 12:09 AM, Alex Harui <[email protected]> wrote: > > 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] > <mailto:[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&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=P0peR5eRbvHOFN1DgfG7q2Lho4cSbH47z3l4m77Vx6s%3D&reserved=0 >> >> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FWeb%2FJavaScript%2FEventLoop&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=P0peR5eRbvHOFN1DgfG7q2Lho4cSbH47z3l4m77Vx6s%3D&reserved=0> >>> On Sep 13, 2018, at 6:54 PM, Alex Harui <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Is setTimeout guaranteed to run after the CLICK event? >>> >>> On 9/13/18, 6:14 AM, "[email protected] <mailto:[email protected]> >>> <mailto:[email protected] <mailto:[email protected]>>" <[email protected] >>> <mailto:[email protected]> <mailto:[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&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&reserved=0 >>> >>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&reserved=0><https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&reserved=0 >>> >>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&data=02%7C01%7Caharui%40adobe.com%7C9ee09de039e5447493e208d619ba7b44%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636724686077133255&sdata=lOu9EWZnwy%2Fz6ENAoLgQp21XdIlyHrY3JEIwLg48J2w%3D&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]> >>> <mailto:[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
