Bug #2850
Bug #2677: fix drawing and functional differences between P2J GUI and 4GL GUI
menu/simple_sm.p drawing issues in Swing client
100%
History
#1 Updated by Greg Shah over 8 years ago
Using ./menu/simple_sm.p
in the Swing client, Vadim reported:
1. After I press on some menu-item in the on of sub-menus of a menubar, root sub-menu in menubar stays "mouse-entered" after body is hidden.
2. The same scenario: sub-menu child is still displayed as focused after it's parent's body is hidden.
3. At the start of the test the warning "Run-time error ... (3269)" is shown and no menubar is displayed in PROGRESS until the user will press any key, but in P2J empty menubar rectangle is drawn.
#2 Updated by Vadim Gindin over 8 years ago
- File fonts_compare.png added
- File hidden_menu_elems.png added
- Fonts compare of PROGRESS, P2J SWING, P2J WEB. There is an other font for WEB.
- MENU bug of showing hidden menu items
#3 Updated by Vadim Gindin over 8 years ago
- File deleted (
hidden_menu_elems.png)
#4 Updated by Vadim Gindin over 8 years ago
- File deleted (
fonts_compare.png)
#5 Updated by Vadim Gindin over 8 years ago
Greg Shah wrote:
3. At the start of the test the warning "Run-time error ... (3269)" is shown and no menubar is displayed in PROGRESS until the user will press any key, but in P2J empty menubar rectangle is drawn.
The place where that background is drawn is the same as for popup_ext.p
: BorderedPanelGuiImpl.draw()
. I've checked that at the drawing time menubar is hidden and it does not post PaintEvent's
. So there are some other widget posted clipping rectangle, that intersects menubar area.. Once again if the widget is responsible to draw it's background itself, why it is needed to draw some rectangles in BorderedPanelGuiImpl.draw()
?
#6 Updated by Vadim Gindin over 8 years ago
- File menubar_background.png added
I'm trying to understand how the "menubar's" rectangle get into screen clippings, that are drawn in BorderedPanelGuiImpl.draw()
. I recall that at that moment MENUBAR itself is hidden and does not post PaintEvent, so there is some other widget, that posts a PaintEvent with similar rectangle. I.e. right after start, when the warning "Run-time error in WAIT-FOR at line ..." is shown an a messagebar menubar is still hidden but it's background rectangle is drawn:
I've added logging of processed PaintEvent native rectangle and source to TitledWindow.processEvent()
method (line 163). Here is what I've got (top, left, bottom, right):
PE: [0, 0, 41, 2147483647], source=MessageAreaGuiImpl PE: [0, 0, 29, 399], source=MessageAreaGuiImpl PE: [0, 0, 41, 399], source=BorderedPanelGuiImpl PE: [0, 0, 29, 399], source=BorderedPanelGuiImpl PE: [0, 0, 2147483647, 2147483647], source=WindowTitleBar PE: [0, 0, 2147483647, 399], source=WindowTitleBar PE: [0, 0, 2147483645, 2147483645], source=WindowTitleBar$WindowIcon PE: [1, 2, 2147483646, 2147483647], source=WindowTitleBar$WindowIcon PE: [0, 0, 2147483647, 399], source=WindowTitleBar$WindowTitle PE: [0, 0, 17, 399], source=WindowTitleBar$WindowTitle PE: [0, 0, 18, 399], source=WindowTitleBar PE: [0, 0, 13, 15], source=CaptionButton PE: [2, 382, 15, 397], source=CaptionButton PE: [0, 0, 13, 15], source=CaptionButton PE: [2, 364, 15, 379], source=CaptionButton PE: [0, 0, 13, 15], source=CaptionButton PE: [2, 348, 15, 363], source=CaptionButton PE: [1, 19, 18, 399], source=WindowTitleBar$WindowTitle PE: [1, 19, 18, 345], source=WindowTitleBar$WindowTitle PE: [0, 0, 541, 408], source=WindowGuiImpl PE: [23, 4, 484, 403], source=WindowWorkSpace PE: [23, 4, 484, 403], source=BorderedPanelGuiImpl PE: [0, 0, 540, 407], source=BorderedPanelGuiImpl PE: [4, 4, 22, 403], source=WindowTitleBar PE: [485, 4, 514, 403], source=MessageAreaGuiImpl PE: [515, 4, 536, 403], source=StatusLineGuiImpl PE: [485, 4, 514, 403], source=MessageAreaGuiImpl PE: [485, 4, 514, 403], source=BorderedPanelGuiImpl PE: [4, 4, 22, 403], source=WindowTitleBar PE: [5, 6, 20, 21], source=WindowTitleBar$WindowIcon PE: [5, 23, 22, 349], source=WindowTitleBar$WindowTitle PE: [6, 386, 19, 401], source=CaptionButton PE: [6, 368, 19, 383], source=CaptionButton PE: [6, 352, 19, 367], source=CaptionButton PE: [23, 4, 484, 403], source=WindowWorkSpace PE: [23, 4, 484, 403], source=BorderedPanelGuiImpl PE: [0, 0, 541, 408], source=WindowGuiImpl PE: [0, 0, 564, 408], source=WindowGuiImpl PE: [0, 0, 540, 407], source=BorderedPanelGuiImpl PE: [0, 0, 563, 407], source=BorderedPanelGuiImpl PE: [508, 4, 537, 403], source=MessageAreaGuiImpl PE: [538, 4, 559, 403], source=StatusLineGuiImpl PE: [508, 4, 537, 403], source=MessageAreaGuiImpl PE: [538, 4, 559, 403], source=StatusLineGuiImpl PE: [4, 4, 22, 403], source=WindowTitleBar PE: [0, 0, 564, 408], source=WindowGuiImpl PE: [4, 4, 22, 403], source=WindowTitleBar PE: [4, 4, 22, 403], source=WindowTitleBar PE: [0, 0, 564, 408], source=WindowGuiImpl PE: [4, 4, 22, 403], source=WindowTitleBar PE: [4, 4, 22, 403], source=WindowTitleBar
Menubar rectangle starts at top=23px approximately, so I don't see here exact rectangle, but here are some intersecting rectangles and some rectangles seems strange:
PE: [0, 0, 41, 2147483647], source=MessageAreaGuiImpl
PE: [0, 0, 29, 399], source=MessageAreaGuiImpl
PE: [0, 0, 41, 399], source=BorderedPanelGuiImpl
PE: [0, 0, 29, 399], source=BorderedPanelGuiImpl
Message area resides in the bottom of the window. It looks that rectangles are incorrect. What do you think?
#7 Updated by Constantin Asofiei over 8 years ago
Vadim, in which branch are you working this?
Also, please remind me, have you managed to address the sub-menu body repaint I mentioned some time ago?
#8 Updated by Constantin Asofiei over 8 years ago
Vadim, I think I know why you are seeing the top light-gray space. Menu-bar gets attached to window (and window is relayout) BEFORE making it visible in GUI. At the time this code is called and warning is shown:
if(!hasEnabledFrame()) { warnNoEnabledFrames(wnd); } wnd.getMenubar().setVisible(true);
the window is already layout as if a menu-bar is added. Only later the menu-bar is being drawn (as it is being made visible).
Can you check what happens if the menubar gets set to not-visible via a trigger? Is the window workspace resized and the menubar hidden? If so, the window needs to layout and use the menubar only if the menubar is visible; and if the menubar's visibility gets changed, the window needs to be relayout.
#9 Updated by Vadim Gindin over 8 years ago
Constantin Asofiei wrote:
Vadim, in which branch are you working this?
bracnh 2677a, revno 11067.
Also, please remind me, have you managed to address the sub-menu body repaint I mentioned some time ago?
You probably mean the note 22 in the task #2736. Yes, I'd tried that, but unsuccessfully: sub-menu's aren't drawn. You can see details about it in that task #2736 the latest post (popup_ext.p). More over I think there is another reason in that task.
#10 Updated by Vadim Gindin over 8 years ago
- File 1123.diff added
Constantin. You were absolutely right about the reason. I've added isVisible
check for MENUBAR to WindowLayout
and fixed dynamic attach/detach case for GUI and CHUI. By the way, MENUBAR can be shown only by assigning to window:menubar
attribute. visible
attribute is not setable for menu. I've added corresponding error. I've also fixed CHUI layout for dynamic case.
Attached diff contains my current changes.
#11 Updated by Greg Shah over 8 years ago
Code Review 1123.diff
1. I don't think it is a good idea to require that the status line and message line implementations must calculate whether or not the menubar exists to determine their location. The containing window should have a method that reports the correct location for each one. Actually, I don't understand why the location changes when the menubar is present. The screen dimensions should not be different and the status/message lines are at the same location no matter if the menubar exists or not.
2. You have mixed in your other changes (right mouse issues and toggle-box issues). I don't think the right mouse changes are safe, so these cannot be included.
Constantin: please review.
#12 Updated by Constantin Asofiei over 8 years ago
- in ChUI, message area, status area, frames (and menubar) are all attached to the same parent (in other terms, they are all siblings). So the change in
WindowChuiImpl
doesn't look correct, as thecontentPane
will contain everything, not just frames - in ChUI, we don't have (yet) a container separation between frames and message area/status area/menubar. This requires more testing, to determine if a frame can overlay the last 3 rows (message + status) in the screen. If it can not, then ChUI needs a "workspace" equivalent, as GUI has, to separate the frames.
- as Greg mentioned, the location/height of the status/message area is constant. Only the height available for frame placement is reduced when the menubar is attached in ChUI.
#13 Updated by Vadim Gindin over 8 years ago
So should I fix it now or postpone that?
#14 Updated by Greg Shah over 8 years ago
1. Separate out the changes for toggle-box/right mouse issues.
2. Fix the rest based on the code reviews.
#15 Updated by Vadim Gindin over 8 years ago
- File 1124.diff added
All 3 points in this task are fixed. See attached diff. I've separated issues from other tasks.
#16 Updated by Greg Shah over 8 years ago
- Assignee set to Vadim Gindin
Code Review 1124.diff
My only question is regarding the addition of the Window.currentFocus()
code. This seems wrong. If I am understanding it properly, as long as there is ever a popup menu in the widget list for a window, only that popup menu will ever be focusable.
Please explain.
#17 Updated by Vadim Gindin over 8 years ago
Greg Shah wrote:
Code Review 1124.diff
My only question is regarding the addition of the
Window.currentFocus()
code. This seems wrong. If I am understanding it properly, as long as there is ever a popup menu in the widget list for a window, only that popup menu will ever be focusable.Please explain.
It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.. I can also move it at this point from the current update to #2849 changes set.
P.S.
Another bug for CHUI: Focused element of MENUBAR must be drawn as Color.MESSAGES
, but it is drawn as Color.NORMAL
. I recall that Color.MESSAGES=Color.NORMAL.withReverse()
.
#18 Updated by Greg Shah over 8 years ago
It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.
I have 2 concerns:
1. When the popup menu is not open, the currentFocus()
will be broken.
2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.
#19 Updated by Vadim Gindin over 8 years ago
Greg Shah wrote:
It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.
I have 2 concerns:
1. When the popup menu is not open, the
currentFocus()
will be broken.
In that case w.isVisible()==false
and therefore super.currentFocus() will be called. Isn't it?
2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.
The difference is in the fact, than Menu
holds it's own widgets tree of menu widgets, that are also focusable.
#20 Updated by Greg Shah over 8 years ago
In that case w.isVisible()==false and therefore super.currentFocus() will be called. Isn't it?
Yes, you're probably right.
2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.
The difference is in the fact, than Menu holds it's own widgets tree of menu widgets, that are also focusable.
The key point here is that we don't want to spread widget-specific behavior (e.g. for menus) into other non-related classes (e.g. window). It makes it hard to understand how menus work. It makes it hard to find all the behavior related to menus. It makes those classes very dependent upon one another, which makes things fragile and more likely to break.
Constantin/Hynek: thoughts?
#21 Updated by Hynek Cihlar over 8 years ago
Greg Shah wrote:
In that case w.isVisible()==false and therefore super.currentFocus() will be called. Isn't it?
Yes, you're probably right.
2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.
The difference is in the fact, than Menu holds it's own widgets tree of menu widgets, that are also focusable.
The key point here is that we don't want to spread widget-specific behavior (e.g. for menus) into other non-related classes (e.g. window). It makes it hard to understand how menus work. It makes it hard to find all the behavior related to menus. It makes those classes very dependent upon one another, which makes things fragile and more likely to break.
Constantin/Hynek: thoughts?
I think Greg is right. Menu should be rendered in its own top-level window, otherwise we will have the same issue as with combo drop-down - the menu won't be able to draw outside of its logical parent window.
#22 Updated by Vadim Gindin over 8 years ago
OK, I understand.
I have collected changes for 2-3 tasks and I would want to commit some of them, that are approved. All issues in this task are approved. Can I commit them (the latest diff) excluding Menu
change?
Talking about ability to draw menu with opened sub-menus exceeding window sizes, is there a separate task for this already? Is it solved for combo-box or other widget, that has the same issue?
#23 Updated by Hynek Cihlar over 8 years ago
Vadim Gindin wrote:
Talking about ability to draw menu with opened sub-menus exceeding window sizes, is there a separate task for this already? Is it solved for combo-box or other widget, that has the same issue?
There is an issue for combo-box drop-down, see #2837.
#24 Updated by Vadim Gindin over 8 years ago
- File 2850_2511.diff added
Here is the latest diff on merged branch.
#25 Updated by Vadim Gindin over 8 years ago
Yesterday, I've fixed CHUI bug with menubar:
Another bug for CHUI: Focused element of MENUBAR must be drawn as Color.MESSAGES, but it is drawn as Color.NORMAL. I recall that Color.MESSAGES=Color.NORMAL.withReverse().
So once again can I commit the last change (diff in the note #24)?
#26 Updated by Vadim Gindin over 8 years ago
- File 2711_rev10976.diff added
Here is the current changes diff (base on revision 10976 of the branch 2677b). Changes solve 3 bugs, described in the note 1 of this task. Please review and approve commit.
#27 Updated by Greg Shah over 8 years ago
Go ahead and check these into 2677b.
#28 Updated by Vadim Gindin over 8 years ago
Committed with revno 10978.
#29 Updated by Greg Shah over 8 years ago
Are there any issues mentioned in this task which are not yet fixed AND which are not part of some other open task?
#30 Updated by Vadim Gindin over 8 years ago
#31 Updated by Greg Shah over 8 years ago
CHUI. Dynamic attach/detach menubar (MENUBAR layout problems). It correlates with the task #2557
It was my understanding that this was a GUI-only issue as noted in #2557.
CHUI. Fix pop-up menu layout (popup-ext.p).
This is still broken? Is it already being worked in another task?
If it is broken and not being worked, please create a new task for this and post the task number here.
I plan to close this task once that task is created, unless you know of a reason I should not.
#32 Updated by Vadim Gindin over 8 years ago
I've created the task #2933 for CHUI layout errors.
#33 Updated by Greg Shah over 8 years ago
- % Done changed from 0 to 100
- Status changed from New to Closed
#34 Updated by Greg Shah over 7 years ago
- Target version changed from Milestone 12 to GUI Support for a Complex ADM2 App