Community
Participate
Working Groups
Use case: User wants multi-page tabs to be on the top, instead of the bottom. This is trivially possible if one could override createContainer. However, the method is currently private. Moreover, the createPartControl is final, so one cannot override that either. I believe the preferred solution would be to make createContainer protected instead of private. Or at least allow a style bit to be passed into the part.
There are currently no plans to work on this feature. PW
Changes requested on bug 193523
*** Bug 251736 has been marked as a duplicate of this bug. ***
Created attachment 115958 [details] Patch to allow tabs to show at the top of the editor Here is a patch that does the job by adding a new constructor. Please review.
This sounds like a simple change, could someone please have a look at it? BTW bug 257767 has some more suggestions on how to allow different kinds of customizations for MPE.
(In reply to comment #5) > This sounds like a simple change, could someone please have a look at it? > BTW bug 257767 has some more suggestions on how to allow different kinds of > customizations for MPE. I wouldn't add a new constructor, although it is probably semantically correct. The general concern with this bug is how to customize the editor, given that we don't expose the MPEP container (and don't want to expose that it's a CTabFolder). Even it we add something like: protected void setStyle(int style) where you can set TOP or BOTTOM, I'm not sure that adding a method where we then say "This will only work until we change the implementation, and then it won't work any more" is a good idea. PW
Would making createContainer protected be considered exposing the implementation?
(In reply to comment #7) > Would making createContainer protected be considered exposing the > implementation? Yes ... as soon as it is protected, it becomes part of the API which means (in theory) it cannot change and must always work. PW
Sure, it becomes part of the API, but I wouldn't consider that as a bad thing. What I wondered is if we could change the return type from CTabFolder to Composite along the way. This would hide the implementation (the fact that MPEP uses CTabFolder) but MPEP would continue to work. Of course, overriding getContainer and returning something different than CTabFolder without overriding other methods will not work but I thought there're such cases present in the API already (e.g. StructuredViewer.getSelection() is supposed to return an instance of IStructuredSelection, AbstractTreeViewer.setContentProvider() expects an instance of ITreeContentProvider, etc.).
(In reply to comment #9) > Sure, it becomes part of the API, but I wouldn't consider that as a bad thing. > What I wondered is if we could change the return type from CTabFolder to > Composite along the way. This would hide the implementation (the fact that MPEP > uses CTabFolder) but MPEP would continue to work. Of course, overriding > getContainer and returning something different than CTabFolder without > overriding other methods will not work There-in lies the problem. Anyone that used this would still be required to know that the internal implementation is a CTabFolder and if that changes, they would be broken. To be able to plug in a new "Container" the work would have to be done to turn the entire container into API in some extendable way ... that's design work as opposed to simply making a method protected. > but I thought there're such cases > present in the API already (e.g. StructuredViewer.getSelection() is supposed to > return an instance of IStructuredSelection, > AbstractTreeViewer.setContentProvider() expects an instance of > ITreeContentProvider, etc.). This is a little different, as they're all API and the classes involved are specializations of the base classes/interfaces. ex: IViewer sets an IContentProvider, and ITreeViewer sets an ITreeContentProvider (both extensions of the base API). The API doesn't expose internal implementation details. PW
OK then, let's go back to the idea of setStyle (or getStyle, called directly from createContainer?). Assuming that allowing people to use any style constants may lead to problems you described, why not at least allow them to choose whether the tabs should be at the top or at the bottom? Both existing implementations of tab folder (TabFolder and CTabFolder) allow it and any custom implementation could probably easily support it as well. As a last resort, the javadoc could say that this is just a hint and doesn't have to be respected by implementation (as it is often case with style flags in SWT). We could have a method like setTabsOnTop (or isTabsOnTop called directly from createContainer?) but personally I think that setStyle (getStyle?) would be better in case we want to support more flags in future.
(In reply to comment #11) > OK then, let's go back to the idea of setStyle (or getStyle, called directly > from createContainer?). Assuming that allowing people to use any style I'd consider a patch + test for this. PW
Created attachment 186879 [details] Patch adding containerStyle field to MPEP The patch also adds two protected methods: getContainerStyle() and setContainerStyle(). Currently setContainerStyle() filters out all style flags but SWT.BOTTOM and SWT.TOP, although this can be easily changed in future. When both are specified, only SWT.BOTTOM is kept. createContainer() still adds SWT.FLAT to the flag specified. Change the patch to your liking or let me know if you think it can be improved. I wasn't sure about the nature or location of the test you asked for. Can you point me to an existing similar test?
(In reply to comment #13) > > I wasn't sure about the nature or location of the test you asked for. Can you > point me to an existing similar test? org.eclipse.ui.tests.multipageeditor.MultiVariablePageTest (and other classes in that package) in org.eclipse.ui.tests PW
Created attachment 189841 [details] Add getContainerStyle v03 This follows the same pattern as ContentOutlinePage. Subclasses can override a getContainerStyle() to provide SWT.TOP|SWT.FLAT. Michal, could you please check this out and make sure it meets your requirements? M6 is a week away, and that's API freeze :-) PW
This'll have to wait until I get a 3.8 milestone. PW
