FWD » Core Development » User Interface

Changes in dynamic MENU, SUB-MENU and MENU-ITEM widgets through the widget attributes are not propagated to the client. Reason, dynamic menus are not registered in LogicalTerminal.menuRegistry and so are not picked up by LT.pushMenuDescrInt.

Also consider removing the methods MenuItemWidget.findMenuItemStatic and SubMenuWidget.findSubMenuStatic. These methods seem they are no longer needed after the changes in #3513.

The propagation should be checked from client to server, too: the mnemonic and preprocessedLabel are exposed via PREPROCESSED-LABEL and MNEMONIC FWD extension attributes, and these are computed by the client-side (this is the original case from which I identified this issue).

Is this task needed for embedded mode web client support?

Greg Shah wrote:

Is this task needed for embedded mode web client support?

Yes, as I don't have access to the preprocessed label, and the module name will include any & characters. Plus, there are runtime implications for any client-side computed attributes not being available on the server-side.

Code review 3582a 11272.

MenuContainerWidget.listMenuTree() is missing javadoc.

In MenuItemWidget.ctor:
if ("RULE".equalsIgnoreCase(subtype) || "SKIP".equals(subtype)), the second equals should be equalsIgnoreCase, too, I believe.

Otherwise the changes are OK.

I'v been using the uast/menu test cases and some of the large GUI apps. If you miss any, please add them to uast/menu.

Code Review Task Branch 3582a Revision 11273

I'm good with the changes. Here are two (very minor) comments:

1. In DynamicWidgetFactory.create() the code LogicalTerminal.registerWidget() call does not need to call h.unwrapWidget() again, it can just use w. This was an existing minor issue with the code, but we might as well fix it now.

2. In MenuContainWidget.listMenuTree(), instead of creating a new list with each recursive call, why not pass the list into the first call and keep passing it down the recursion tree? This would be more efficient.

I don't think you need to redo all testing for these changes. If the basic functionality works then it is fine. I don't think this code can really be hit in ChUI regression testing, so I am inclined to merge it to trunk once you have these last changes made and minimally tested.

What do you think?

Greg Shah wrote:

I don't think you need to redo all testing for these changes. If the basic functionality works then it is fine. I don't think this code can really be hit in ChUI regression testing, so I am inclined to merge it to trunk once you have these last changes made and minimally tested.

Thanks, I've fixed your notes in 3582a 11275. And yes, I agree, it can be merged to trunk.

Please merge 3582a to trunk.

3582a was merged to trunk rev 11272 and archived.