FWD » Core Development » User Interface

If the current focus is in the menu bar, then pressing ESC must move the focus input to the window root frame. The current P2J doesn't traverse the current focus correctly from the menu bar to the root frame. This test case can be used to reproduce this issue

/*
1) Open subMenu1
2) Press ESC
5) subMenu1 is closed
6) Press ESC
7) subMenu1 in menu bar becomes unselected
8) Press ESC
9) Message "f1 trigger" is printed.
*/
DEFINE SUB-MENU subMenu1
MENU-ITEM subMenuItem1 LABEL "menu-1-item-1" 
MENU-ITEM subMenuItem2 LABEL "menu-1-item-2" 
MENU-ITEM subMenuItem3 LABEL "menu-1-item-3".

DEFINE MENU mbar MENUBAR
SUB-MENU subMenu1 LABEL "Sub menu 1".

define button bt1 LABEL "Button 1".

define button bt2 LABEL "Button 2".

DEFINE FRAME f1 bt1 skip bt2  WITH SIDE-LABELS.

enable all with frame f1.

ASSIGN CURRENT-WINDOW:MENUBAR = MENU mbar:HANDLE.

ON ENDKEY, END-ERROR OF frame f1 DO:
   message "f1 trigger".
   RETURN NO-APPLY.
END.

WAIT-FOR CLOSE OF CURRENT-WINDOW.

Created task branch 3538a and merged in changes from 3487b.

3538a rebased against trunk revision 11252.

Due to rebasing issues in branch 3538a I created 3538b and merged in all the changes from 3538a. 3538a will be dead-archived.

Due to rebasing issues in branch 3538b I created 3538c and merged in all the changes from 3538b. 3538b will be dead-archived.

Please review 3538c, it resolves this issue plus multiple others - several menu-related abends, drawing issues, event processing issues, plus some related changes and code cleanups.

The branch has passed GUI regression tests. ChUI regression testing is in progress.

Code Review Task Branch 3538c Revision 11258

I'm good with the changes.

If I understand correctly, this also resolves #3487, right?

What about #3554?

Greg Shah wrote:

Code Review Task Branch 3538c Revision 11258

I'm good with the changes.

If I understand correctly, this also resolves #3487, right?

What about #3554?

Yes, #3487 and #3554 are also resolved by 3538c. Btw. I have some more upcoming changes which will require another round of review.

Please review latest 3538c. It contains an overhaul of the menu navigation implementation. It was simplified in order to resolve the remaining menu issues.

The branch passed GUI regression tests (Hotel GUI, selected uast tests, large customer GUI app 1, large customer GUI app 2), ChUI regression tests are running.

No more changes are expected to be checked in unless any regressions are discovered.

Code Review Task Branch 3538c Revision 11271

These changes certainly make the menu processing significantly simpler. Well done!

1. In MenuItem.processKeyEvent() line 306 calls getParentMenu() and saves the result in the menu local variable. It is used in the if condition on line 326 and then inside the true branch, the getParentMenu() is called again. Is this intentional or can we safely reuse the menu reference?

2. Menu.hasMenuFocus() is missing javadoc.

Do you expect to finish testing today? I want to get this into trunk.

Greg Shah wrote:

1. In MenuItem.processKeyEvent() line 306 calls getParentMenu() and saves the result in the menu local variable. It is used in the if condition on line 326 and then inside the true branch, the getParentMenu() is called again. Is this intentional or can we safely reuse the menu reference?

2. Menu.hasMenuFocus() is missing javadoc.

Both are fixed in 3538c 11273.

Greg Shah wrote:

Do you expect to finish testing today? I want to get this into trunk.

I haven't finished ChUI regression tests, I need to rule out several failures that are most likely negatives. I had one run stuck in Ctrl-C part and so the testing got delayed. I will get results from the current run by tomorrow morning local time.

3538c passed ChUI regression tests. It is rebased against latest trunk. I also checked in one other simple change in rev 11275, which only affects GUI. Once reviewed the branch is ready to be merged to trunk.

3538c merged to trunk as revision 11255. 3538c was archived.