Community
Participate
Working Groups
1. Double-click a file in the 'Package Explorer' to open it. 2. Start typing. The document is not modified. 3. Notice that the text caret is not in the editor.
Yep, thanks Remy. Does anyone know what the E4 (MContributedPart) equivalent is to the 'setFocus' method?
(In reply to comment #1) > Yep, thanks Remy. Does anyone know what the E4 (MContributedPart) equivalent is > to the 'setFocus' method? Is it MPart's setActiveChild(P) method?
No, that's how a parent identifies which of its children is active (== the selected tab in CTabFolders). I guess it's some sort of listener on the Context for the WorkbenchWindow, there can be many tab folders, each with their own 'activeChild' but only one active part for a given WBW...
(In reply to comment #1) > Does anyone know what the E4 (MContributedPart) equivalent is > to the 'setFocus' method? I'm not sure I understand the question. Are we intending on modelling focus?
I don't think so. However we do have some concept of 'activate' in E4 but, as this bug points out, we don't actually set the keyboard focus when a new view/editor is opened and 'activated' or the user clicks on a -tab- to activate a part (if they click on the actual part the focus is set by the platform). In 3.x one of the 'required' methods for a view/editor to implement is 'setFocus'. This provides the workbench an API hook to call whenever it decides that a particular part should gain the focus (which it uses when that part is activated) while allowing the implementing code to determine which of its own controls should really gain the keyboard focus. This is a necessary abstraction since only the part itself knows where it thinks the real focus should go. What code needs to be added to the PartFactory#activate method in order to fix this defect for E4 Parts ? Since there are no constraints on the implementation class for an E4 part we lack any specific API to solve the problem. We could: - have E4 support some life-cycle events of which one would be 'part activated' and -require- that all implementations add a listener and respond accordingly when they become active. While this would likely work but it places the onus on the developer to 'know' that he has to do this. - take the 3.x route and require that all contributed part implementations derive from a specific base class or implement some specific interface. This is likely a problem for non-java contributions but works well in terms of leading the developer of a new view by providing the abstract methods so he knows he has to provide the implementation for them. - use introspection to find a 'setFocus' method and use it if it's there. More general but still involves the part author to 'know' he should be providing one.
I prefer the 3.x option though that may be because I'm used to 3.x code. In Kai's demo he has an ExitHandler class which defines an execute(IWorkbench) method. However, this class doesn't actually subclass anything nor does it implement any interfaces. I'm not sure how he knew the right method signature to define, which I guess kind of goes back to how the other two options requires the developers to just magically become aware of these requirements. The 3.x option allows compile-time error checking whereas this reflection thingamajigger will only be discovered at runtime. On the other hand, enforcing a type hierarchy constraint can potentially prevent code reuse if someone was planning to subclass something else.
(In reply to comment #6) > On the other hand, enforcing > a type hierarchy constraint can potentially prevent code reuse if someone was > planning to subclass something else. Worse even, it can prevent people from using other languages, which is the main reason for not using subclassing in cases like this. I have seen many examples of subtle bugs that were caused by clients who did not implement setFocus() correctly. In e4, I would like us to call setFocus on the control ourselves, with a way to override it if clients need full control.
(In reply to comment #7) > I have seen many examples of subtle bugs that were caused by clients who did > not implement setFocus() correctly. In e4, I would like us to call setFocus on > the control ourselves, with a way to override it if clients need full control. We can provide a deterministic default behaviour (for SWT at least). Composite#setFocus() will set the focus to the first child that can take it. I think it's reasonable for us to simply setFocus to the parent composite unless the part explicitly says it will take responsibility. PW
OK...the question now becomes "How does a part explicitly say it will take responsibility"? We've just punted the question. Were this an acceptable solution I'd have expected WorkbenchPart to have this as its default implementation rather than declaring the method 'abstract'...
> OK...the question now becomes "How does a part explicitly say it will take > responsibility"? Two options come to mind: 1) Class can optionally implement IFocusable (I didn't even know we had this!) 2) Use an annotation to denote the method to be called when focus is taken. This is similar to the annotation strategy we use to identify injection-related methods (@In, @PreDestroy, @PostConstruct, etc). I can't think of a clear value of annotations here, so 1) is probably easiest. The key is that these are optional extras that a part class doesn't need to implement. This avoids the clutter for simple cases and allows use of different languages.
Sounds ok but I was wondering why we wouldn't use introspection for the 'setFocus()' method and call it if it was there, this is even more loosely coupled since it doesn't require the interface.
We can use introspection, but I think there should still be an interface available (like we have IEclipseContextAware for the injection stuff). Having an interface to implement makes it more explicit and less "magic" for clients, and gives us a place to describe the API contract. Some clients may want to avoid referencing Eclipse APIs and just declare the method without implementing the interface, but others won't have such a limitation and will prefer implementing the interface to make their code clearer.
I haven't thought about the implications, but to regularize cases like this, we could use "DuckType" or "RelaxedDuckType", which we could copy from the data binding examples plug-in. (These classes were put there by Dave Orme with enormous foresight apparently ;-) Can be used as follows: Object viewImplementation = ...; if (DuckType.instanceOf(IFocusable.class, viewImplementation)) { IFocusable focusable = DuckType.implement(IFocusable.class, viewImplementation); focusable.focus(); } else { // do something else }
Created attachment 142069 [details] Patch to initially 'activate' new editors and set the focus This is just some interim work, we're not using the WorkbecnhPage#activate yet which is likely the right place to put this code...
Committed in >20090720. Applied the patch. At least opening an editor will activate it correctly and set the focus now.
Created attachment 142119 [details] Update the editor actionbars on an editor activate v01 This is the code that correctly updates EditorActionBarContributors when an editor is activated ... I'm not sure where it should live. Eric, feel free to move it around, I haven't committed it. PW
Created attachment 142142 [details] Makes WorkbenchPage.activate work Paul, I've added the SubActionBars code to the 'activate' method, looks like the right place. Also switches 'openEditor' to use the newly implemented 'activate', I'll see where I should 'activate' views next...
Committed in >20090721. Applied the patch. Now on to Views...
See bug #284141 which may be related
Eric, is there more of this missing or is it done? PW
Still doesn't work, at least for me...
Already fixed. PW
