Community
Participate
Working Groups
3.4 M1. I could not find a listener that informs when the active page of a MultiPageEditorPart changes.
Paul, M6 is API freeze, so what should tagging this to M7 tell us? ;-)
Let's look at this in M3 PW
Hi Paul, was looking at something else yesterday and noticed there's an IPageChangeProvider interface from JFace. UA's FormEditor already implements the interface (and FormEditor in turn extends MPEP), maybe it is worth implementing IPCP in MPEP? I don't know what kind of API connotations doing this may have though. Will clients break because of this? I have opened bug 284384 about IPCP's "dialog restriction".
(In reply to comment #3) > I have opened bug 284384 about IPCP's "dialog restriction". Sorry, this should be 248384.
Created attachment 114562 [details] MPEP page change notification v01 First pass at a page change notification API. I've followed in the footsteps of the FormEditor, and arranged the API in a way that will hopefully be easy for us to push down the FormEditor functionality to the MPEP. The selected page and page change events of the API contain the active page index. Specific MPEP subclasses can expose API to allow the page to be retrieved, or substitute their own selected page to be used in page change events. PW
(In reply to comment #5) > Created an attachment (id=114562) [details] > MPEP page change notification v01 Haven't applied this myself but I've got a small comment about the getSelectedPage() method. Its javadoc needs to be fixed since it says "Maybe be null.".
(In reply to comment #5) > The selected page and page change events of the API contain the active page > index. Specific MPEP subclasses can expose API to allow the page to be > retrieved, or substitute their own selected page to be used in page change > events. The other alternative for the default MPEP would be to return an IEditorPart from getSelectedPage() if it is an editor or the Control if it is not. I'm not sure which would be best for use of this listener, although I'm leaning towards the Integer. PW
Dani, would this API (based on org.eclipse.jface.dialogs.IPageChangeProvider) be sufficient for page change notifications? Adam, the API is set up so that your implementation of getSelectedPage() would be used (and fill in the IForm). That's the good news :-) This API allows a PageChangeEvent where the new page is null, but I noticed that you don't fire a page change event in that case ... will that cause you problems? The other open issue: getSelectedPage() currently returns an Integer which (with the right subclass of MPEP) can give you access to the page you want. The other option would be to return the "page", IEditorPart or Control. That would be consistent with how the API is used in dialogs and how FormEditor currently uses it. Maybe a protected method to allow the page index to be looked up would make the second option just as good as the first. PW
>Dani, would this API (based on org.eclipse.jface.dialogs.IPageChangeProvider) >be sufficient for page change notifications? It's a bit laborious to use because I have to add/remove the listener to the MPE each time one (or some other editor kind) gets (de-)activated. I would have preferred some sort of IPartListener3 that sends me the notification. Note regarding the new API: MultiPageEditorPart.getSelectedPage() should specify the return type in the Javadoc.
Created attachment 115595 [details] Alternate page change API v01 This is an alternate page change approach. Involve the IPartService and an IPartListener3 (this is simply the API, not the implementation). public void partPageChanged(IWorkbenchPartReference partRef, Object selectedPage); I thought I would explore allowing any part the ability to fire page change events, and simply funnel them all through the IPartService. Then hook up the MPEP and PageBookView by default and away we go. But I don't like it. I'm still firing all of the events, but they're going to all listeners instead of specific ones (I could live with that, though). But it ends up touching way more interfaces than I would like, and being hooked up through the PartService/PartListener classes as well. I have concerns about evolving this API into the more componentized e4 form, as it would need to be published in a service so that it could be retrieved from its parent (as opposed to globally). While less convenient, having the MPEP implement the interfaces and fire the events is cleaner and more localized. PW
Couldn't we simply add an IPartListener2Extension? That way no new methods would be needed on IPartService and it is more intuitive: having an IPartListener3 with no part* events looks a bit strange. If you prefer the IPartListener3 approach then it should extend IPartListener2 for completeness.
(In reply to comment #8) > Adam, the API is set up so that your implementation of getSelectedPage() would > be used (and fill in the IForm). That's the good news :-) This API allows a > PageChangeEvent where the new page is null, but I noticed that you don't fire a > page change event in that case ... will that cause you problems? I don't think it would be forms that this could cause problems for. There are probably consumers of the API that are not using null guards since we are protecting them. I suspect if this changed it would break some people in the short term. The fix for them will be trivial though, so I think the sooner we get this in, the sooner they can find and fix the problems.
(In reply to comment #11) > Couldn't we simply add an IPartListener2Extension? That way no new methods > would be needed on IPartService and it is more intuitive: having an > IPartListener3 with no part* events looks a bit strange. I have no real problem with the IPartListener3 itself extends IPL2 (that's just extending our framework). And IPL2 doesn't inherit from IPL for a specific reason that wouldn't effect IPL3 inheriting from IPL2 (enough acronyms). My concerns are with the direction, i.e. 1) dealing with the IPartService at all, 2) with the need to provide API so that a page-changing part could change pages, 3) supporting that forward into e4 and 4) that it's not compatible with the FormEditor. We had a discussion at the UI call and I think IPageChangedProvider is the preferred approach here. PW
I like the idea of compatibility with forms. =p +1 IPageChangedProvider.
Created attachment 115757 [details] MPEP page change notification v02 The page change notification refined. getSelectedPage() will now return the IEditorPart or Control, similar to the FormEditor returning the IFormPage and consistent with other uses of this API. FormEditor can still override the getSelectedPage() method to return their IFormPage. getActivePage() is public and will return the page int if a client needs it. PW
(In reply to comment #14) > I like the idea of compatibility with forms. =p > > +1 IPageChangedProvider. Same. Page changes also seems too specific for...specific parts, for the lack of a better adjective. I mean, page changes only apply to parts that has pages and introducing a new IPartListener somewhat implies that it's applicable to all parts (upon first glance and without looking at its declared methods). I'm not sure if that came out right but I think you all know what I was trying to say.
What I had in mind was using the latest patch PLUS specifying in IPartService.addPartListener(IPartListener2) that the given part can optionally also extend IPageChangedListener in which case my listener would also be notified on page changes i.e. deal with the labor of listening for part (de-)activation inside the framework instead of loading it onto the clients.
(In reply to comment #17) > What I had in mind was using the latest patch PLUS specifying in > IPartService.addPartListener(IPartListener2) that the given part can optionally > also extend IPageChangedListener in which case my listener would also be > notified on page changes i.e. deal with the labor of listening for part > (de-)activation inside the framework instead of loading it onto the clients. While I'd consider this, it makes sense to me that the provided IPartListener2 that optionally extends IPageChangedListener would be notified about all page changes for all parts that implement IPageChangeProvider (whether they're active or not). That would be the advantage of going to the window level service, I think. PW
Created attachment 115965 [details] MPEP page change notification available in IPartService v03 This implementation allows your IPartListener2 to also implement IPageChangedListener. It will receive PageChangedEvents for all parts that have been instantiated in the page, and works at both the page and window level. Dani, is this sufficient? PW
>Dani, is this sufficient? Paul, the patch works fine for me. Thanks!
(In reply to comment #19) > Created an attachment (id=115965) [details] > MPEP page change notification available in IPartService v03 Added some javadocs to IPartService and MultiPageEditorPart and released into HEAD >20081024 I've opened bug 252023 to consider doing the same for PageBookView. PW
In I20081027-1800 PW
