| Summary: | [FastView] Fast views do not always close on Escape | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Ralf Sternberg <rsternberg> |
| Component: | UI | Assignee: | Eric Moffatt <emoffatt> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, eclipse, john.arthorne, markus.kell.r, pbenedict, pwebster, tomasz.zarna |
| Version: | 4.2 | Flags: | john.arthorne:
review+
|
| Target Milestone: | 4.4 M1 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | candidate43 | ||
| Bug Depends on: | |||
| Bug Blocks: | 414050 | ||
|
Description
Ralf Sternberg
I've seen this one and it really annoys me, I'll take a look this is a function of the page not being able to select some other part to activate (which would cause the FV to close). They should also open on 'enter' for accessibility... *** Bug 384814 has been marked as a duplicate of this bug. *** I've marked Bug 384814 as a DUP because they all share the underlying fault that we were basing our 'tear down' strategy on some other part becoming active. The current code searches for a part to activate in the current presentation (i.e. not in the trim). While usually not an issue if we have an *empty* shared area which is maximized then there are no candidates to activate. We need to allow there to be *no* active part for cases like this, I'm just not sure what impact this will have on the rest of the environment (i.e. we can't send a 'partActivated' message because many current listeners would NPE...). *** Bug 386914 has been marked as a duplicate of this bug. *** John, could you please +1 this based on the review we just did ? I still don't have an answer for clicking on the background of an empty editor stack but at least clicking on the icon or hitting ESC will tear down the current stack. commit 4310bfdf8f23e37eb27917b76b746d336b3ed2d9 Fixes at least the ESC and clicking on the open view's icon work... The case of clicking on the background of the empty editor area is *not* fixed by this but this is a much time as we have in 4.2.1. I've re-opened Bug 384814 to track this for future work... Verified in M20120829-1200. (In reply to comment #7) > John, could you please +1 this based on the review we just did ? > > I still don't have an answer for clicking on the background of an empty > editor stack but at least clicking on the icon or hitting ESC will tear down > the current stack. No, hitting Esc to close the fast view still neither works in 4.3 M3 nor 4.2-M20121031-1200. Note: the main reason why 'Esc' is not working is that the fast view does not get the keyboard focus (as compared to 3.x). (In reply to comment #10) > (In reply to comment #7) > > John, could you please +1 this based on the review we just did ? > > > > I still don't have an answer for clicking on the background of an empty > > editor stack but at least clicking on the icon or hitting ESC will tear down > > the current stack. > > No, hitting Esc to close the fast view still neither works in 4.3 M3 nor > 4.2-M20121031-1200. That depends on how you open it. If I click a minimized view to reveal it, and then hit Esc, it minizes for me. Defered to 4.3 This is indeed a pain...in debugging with the editor area maximized the Debug view keeps being 'broughtToTop' but not activating (so no Focus). In this case neither hitting the ESC (as Dani points out) nor clicking on the editor will close the stack. I suspect that I'll have to arrange things so that both of these cases cause the stack to close. Perhaps a traversal listener for ESC and a mouse *filter* (a la 3.x) would work better... Committed: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a23cbd01b2bb406e19fd38cf2fd6e3afcc82e957 This adds a mouse filter when a trim stack opens that will close it whenever a mouse click is detected in the *client area* of the shell but not over the floy out composite. This allows using the mail toolbar / menu but still lets you tear down the fly out by clicking elsewhere). I'm tentatively going to mark this one as fixed...if anybody thinks we *need* ESC to work as well feel free to re-open. Modified the code to use MouseDown rather than MouseUp: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f36ba3c99507d58dfa56979682733451bd2dc3d9 Could this be backported to 4.3.1? The annoyance is great enough and fix is small enough that should be safe. I don't want to wait a year to get this. (In reply to comment #15) The mouseListener on the Display looks very scary, especially since the removeFilter() call is nested in many if-conditions. And indeed, the filter was leaked in the first scenario I tested (open a second workbench window; execute comment 0; click active window's close box): org.eclipse.swt.SWTException: Widget is disposed at org.eclipse.swt.SWT.error(SWT.java:4397) at org.eclipse.swt.SWT.error(SWT.java:4312) at org.eclipse.swt.SWT.error(SWT.java:4283) at org.eclipse.swt.widgets.Widget.error(Widget.java:472) at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:344) at org.eclipse.swt.widgets.Control.getBounds(Control.java:1233) at org.eclipse.e4.ui.workbench.addons.minmax.TrimStack$1.handleEvent(TrimStack.java:120) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1262) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1057) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4170) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3759) ... Moreover, I think you're not trying to fix the right bug, see comment #11: > Note: the main reason why 'Esc' is not working is that the fast view does > not get the keyboard focus (as compared to 3.x). That's the scenario to fix. It should not be possible in the first place to get into the bad state where a fast view is shown but another part has focus in that window. A MouseDown filter should not be necessary for this. +1 for comment 18 The real problem is that fast views don't get the keyboard focus under some conditions, e.g. when running tests opens the JUnit view. Not closing on click outside of the view is only one implication. This bug also makes it impossible to continue from that point using the keyboard. For me this is the most annoying problem in E4 and I'm sure I'm not the only IDE user who prefers using the keyboard. However, not closing on ESC (the title of this bug) is obviously not related to the focus problem, because that also doesn't work when the fast view is focused. It worked in 3.x and it should also work in E4 again. Should we have another bug for the focus issue? Perhaps I'd be better off to perform the same hack we do in 3.x; *active* the view being shown in the minimized stack even if it's not being activated by the code (which is really calling BringToTop). let me take a cur at this... Personally, I like the idea that the FastView doesn't have the focus. For example, I want my Console Output and Error to popup on a message, but I don't want to take control away from my activity. All I want is to click somewhere or continue typing to dismiss the FastView. Right now, I have to click the FastView to give it focus just so I can click somewhere else to get rid of it. OK, thanks for the fast input. I've just committed: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=eed1302da726969deb2d6554f75f9ecfc0bcdb8f This reverts the previous change and instead *activates* the view that's coming up. I was trying to avoid this because of the API issue but this fix is both simpler and also allows ESC to work... Paul, you should be OK even with this change...it'll initially give the focus to the Console but either hitting ESC or clicking somewhere else should work to get back to your main task... Thanks Eric. I think ESC is perfect escape hatch ;-) Back to my question (comment #17), if this solution is considered relatively low-impact, can it be back-ported to 4.3.1? Paul, I'm hoping so... I'm going to re-mark this as FIXED barring further feedback... (In reply to comment #24) > Paul, I'm hoping so... > > I'm going to re-mark this as FIXED barring further feedback... I did not review the fix but verified that it works as expected in N20130729-2000. (In reply to comment #25) > (In reply to comment #24) > > Paul, I'm hoping so... > > > > I'm going to re-mark this as FIXED barring further feedback... > > I did not review the fix but verified that it works as expected in > N20130729-2000. While it works for views, it is still broken for shared/editor area as fast "view". Filed bug 414108 to track that. Verified in 4.4.0.I20130806-2000. 4.5.1: I put Outline view into a FastView. When I click on it, it doesn't hide on Escape click. So +1 from me. |