Some additional explanations:

TabbedContainer implements the Tabbed interface that is very similar to JTabbedPane interface. It is only accessed by
the WindowSystem via this interface, so any subclass of JTabbedPane implementing this interface can easily be used as a
replacement.

Update: Please refer to the "Issue 169099 blocks:" section for the real issues blocked by this (some of the links in the
description are wrong)

Created attachment 85167 [details]
patch

Created attachment 85168 [details]
patch updated

Forgot to hg add in the patch sending again (patch updated).

Created attachment 85169 [details]
added friend packages

[JG01] License notice should be at the very beginning of the file; please fix DefaultTabbedComponentFactory.java
accordingly.


[JG02] Now would be a good time for someone to replace Constants.MODE_KIND_* with an enum, probably as a preparatory patch.

BTW seems like the body of createTabbed could be simplified to

return Lookup.getDefault().lookup(TabbedComponentFactory.class).getTabbedComponent(getKind());

or am I wrong?

CCed also saubrecht and dsimonek. Wait for their review and comments. They are the ones who must accept (or reject) any
changes in Window System. I will ask them to pay attention to this proposal.

BTW, thanks for putting your time into this effort!

Created attachment 85190 [details]
changes per Jesses comments

Hi Tony again,

After consultations with saubrecht, we agreed that TabbedComponentFactory as service provider, impl in separate module
is good approach and we are more then fine to accept that. However, we found some issues and we have following comments
and suggestions.

[daf 01] Whole package o.n.core.windows.ui would be exposed as friend API, I have hard time to accept it, as there is a
bunch of stuff which really shouldn't be exposed.

[daf 02] Anyone who wants to plug their (yet another) implementation of tabs would need to modify core.windows to
declare their module as a friend. Which means convince NB team to modify core.windows so that one can use their tabs
impl in *next version* of NB platform. I think with little changes suggested below we can get rid of such limitation.

[daf 03] Please add full javadoc to TabbedComponentFactory, including @since and specification version (you'll need to
increase spec version of module where TabbedComponentFactory will end up - read below)

[daf 04] Please add API change record to apichanges.xml (to module where TabbedComponentFactory will end up - read below)

[daf 05] Please add package-summary javadoc to "Architecture Description" (arch.xml), section "arch-what" to module
where TabbedComponentFactory will end up - read below. 

And now finally our suggestions:
- create new API package tabcontrol.customtabs (or better name if you have one) in o.n.swing.tabcontrol module
- make that package exposed as public API and make it category "Under development" so that we can still change it if we
need (see http://openide.netbeans.org/tutorial/api-design.html#category-official)
- refactor-move TabbedComponentFactory and Tabbed into tabcontrol.customtabs
- refactor-move DefaultTabbedComponentFactory into org.netbeans.core.windows.view.ui package
- provide some basic javadoc for Tabbed interface (something about "JTabbedPane-like tab component nb specific abstraction")

The rest should stay almost the same I hope, as core.windows already depends on tabcontrol module, so we can
refactor-move Tabbed easily.

Benefits of suggested approach are that we didn't exposed half of core.windows and moreover developers on NB platform
can plug their tabs impl easily, adding their module which depends on o.n.swing.tabcontrol and implements
TabbedComponentFactory and Tabbed.

Although I understand that my comments are putting more work on you, I hope it is worth and please don't feel
discouraged to continue. One example - ideally Tabbed should be abstract class to allow for future expansion IMHO, but
that would mean significant rewrite so I'm saying interface is OK :-)

Looking further to your comments, thank you.

Thanks a lot for your comments. I've already expected that adding new packages to the friends API might be the most
difficult aspect of this. 

I'll try to refactor it, but in my proof of concept I had to use a bunch of classes from core.windows:

import org.netbeans.core.windows.Constants; //needed for Constraints used by TabbedHandler
import org.netbeans.core.windows.ModeImpl; //needed to implement : public boolean inMaximizedMode(Component comp)
import org.netbeans.core.windows.Switches; //to find out if sliding, etc. is enabled
import org.netbeans.core.windows.WindowManagerImpl; //at various places, e.g. to find out if in maximized mode
import org.netbeans.core.windows.actions.ActionUtils; // for Popup actions (AutoHideWindowAction)
import org.netbeans.core.windows.view.ui.slides.SlideController; // implemented by JTabbedAdapter

So it will mean some refactoring beyond your sugestions... 

The Tabbed interface wasn't created by me, and I didn't want to change too much in code created by the NetBeans Team,
but I'll have a look if I convert it to an abstract class. It could eventually solve some of the problems with
dependencies mentioned above, but it would also mean refactoring of TabbedAdapter class ( which extends TabbedContainer
now ) to extend Tabbed and delegate to TabbedContainer.


 

I did the refactorings as suggested and changed my code to replace all Dependencies on core.windows in my proof of
concept. This is the last remaining one:

import org.netbeans.core.windows.WindowManagerImpl;

Needed to find out if my tc is docked:
WindowManagerImpl.getInstance().isDocked(getTopComponentAt(tabIndex));

I'll upload the updated patch, as soon as I've fixed that. If you have an alternative for the above call let me know. 

WindowManagerImpl.getInstance().isDocked(getTopComponentAt(tabIndex));

is more or less equal to

SwingUtilities.getWindowAncestor( getTopComponentAt(tabIndex) ) == WindowManager.getDefault().getMainWindow()

Uff, that was quick. I was thinking about other alternatives and you already solved nearly everything...wow. I'm looking
forward to see the code in your proof of concept to see how you dealt with issues that you pointed out. 

Created attachment 85447 [details]
changes per comments

>daf 01] Whole package o.n.core.windows.ui would be exposed as friend API, I have hard time to accept it, as there is a
bunch of stuff which really shouldn't be exposed.

Done. No changes to o.n.core.windows friend APIs required, the proof of concept already works without. So others impls
should do as well.

>[daf 02] Anyone who wants to plug their (yet another) implementation of tabs would need to modify core.windows to
declare their module as a friend. Which means convince NB team to modify core.windows so that one can use their tabs
impl in *next version* of NB platform. I think with little changes suggested below we can get rid of such limitation.

Done. No more friend dependency required in my proof of concept, so others shouldn't need it either

>[daf 03] Please add full javadoc to TabbedComponentFactory, including @since and specification version (you'll need to
increase spec version of module where TabbedComponentFactory will end up - read below)

Done. Changed Specisfication version to 1.17 and added Javadoc.

>[daf 04] Please add API change record to apichanges.xml (to module where TabbedComponentFactory will end up - read below)

Done. I moved it to package tabcontrol.customtabs in o.n.swing.tabcontrol module and documented changes in
apichanges.xml. Status is under development as far as I've seen, so it should be fine like that. 

>[daf 05] Please add package-summary javadoc to "Architecture Description" (arch.xml), section "arch-what" to module
where TabbedComponentFactory will end up - read below. 

Done. package-summary javadoc added to tabcontrol

I think I've implemented all changes requested, please review. 


proof of concept working with this is available here:

http://kenai.com/projects/nb-tabbedpane

@Dafe:

------- Additional comments from dsimonek@netbeans.org Wed Jul 29 12:17:53 +0000 2009 -------
>I'm looking forward to see the code in your proof of concept to see how you dealt with issues that you pointed out. 

Well, I must admit that most of my efforts were solved by copying. Up to now it's only to check if it seems doable...

I tried to review, but the patch is still the same?

Btw, I was thinking about additional info that will be useful to Tabbed implementors and I found that we already have
this for NB specific tabs and can be reused. Please look at org.netbeans.swing.tabcontrol.WinsysInfoForTabbedContainer
and WinsysInfoForTabbed interfaces, I think this is what we want, I mean this:

public interface TabbedComponentFactory {
    public Tabbed getTabbedComponent(int type, WinsysInfoForTabbedContainer winsysInfo);
}

Tabbed implementors would then use winsysInfo for some of issues you mentioned.

Created attachment 85479 [details]
changes per comments -> second try

@Dafe I've seen WinsysInfo, but it doesn't help: The delegate is in TabbedAdapter. In the end it calls the TabbedAdapter
to ask for it's state, see TabbedAdapter.WinSysInfo:
e.g. this:

 public boolean isTopComponentMaximizationEnabled() {
            return Switches.isTopComponentMaximizationEnabled();
        }

 If I follow this pattern I would have to implement my own WinsysInfo which would result in the same problems (&
dependencies) as before. 

Uf, there must be something that I'm missing...why would you need your impl of WinSysTabbedInfo?
TabbedAdapter.WinSysInfo will give incorrect answers for your tabs? (sorry for being so dumb, I haven't seen the code
for a year and forgot code structure...) I thought that we could move creation of TabbedAdapter.WinSysInfo into
createTabbed() methods when we are lookuping and calling the factory.

Hm, I'm not sure because the code structure is a bit complicated, but I think it's like this: 

When I want to get a WinSysInfoForTabbedContainer I have to call the getDefault method with a WinsysInfoForTabbed as an
argument. The WinSysInfoForTabbedContainer then stores this and delegates method calls to it (e.g.
getOrientation(Component comp), and inMaximizedMode(Component comp)). So I need to supply an implementation of
WinsysInfoForTabbed to get a WinSysInfoForTabbedContainer.

The default implementation in tabcontrol module simply returns true from all other method calls. So it's of no use for
me, because I only get a useful answer for the questions I basically answer myself via my WinsysInfoForTabbed
implementation.

All implementations of WinSysInfoForTabbedContainer that do something useful are in core.windows, e.g. the static inner
class WinsysInfo in TabbedAdapter. Since I don't want to depend on core.windows I can't use it in my module, and I can't
use it in tabcontrol either otherwise I get a circular dependency.

That's why I decided to implement all the stuff I need myself in a simple utility class called WinSysUtil. It's not hard
to get that kind of info anyway. You either need to ask TopComponent for getClientProperty which is totally OK and part
of the API or for the properties of the whole WindowSystem do the same that Switches class in core.windows does: read
the value from a bundle.

Correct me if I'm wrong, but all other solutions would probably mean refactoring moving code from core.windows to
tabcontrol. 

Thanks, now I understand. I'll attach modified patch that explains what I was aiming for. Basically create
TabbedAdapter.WinsysInfo and pass it to the factory...seems to work for me.

Created attachment 85550 [details]
patch with added WinSysInfo

oh, now I get it (sorry). Yes, that makes life a lot easier.

Created attachment 85597 [details]
final touches to api change - javadoc etc.

Hi again, I'm glad that we are in agreement, perfect!

I did some final touches to the patch, ensuring that generated javadoc is OK etc. I also removed changes to friend API
of core.windows that were still part of the patch, they shouldn't be needed anymore, but here I'm not sure so please
check it, thanks.

Otherwise I'm completely satisfied and I accept this API change, it can be integrated in this state IMHO. Good time for
integration will be after weekend when 6.8 Milestone1 is branched. Well perhaps one last minor note - maybe "swingtabs"
would be better name then "othertabs" for module which implements JTabbedPane-based Tabbed impl - but that has nothing
to do with API, so it's not blocking us in any way.

saubrecht and rmichalsky, please review current patch and tell us if you have any objections, thank you. Some short
recapitulation for you: org.netbeans.swing.tabcontrol.customtabs API folder was created and TabbedComponentFactory and
Tabbed interfaces was added there. API allows its clients to write own implementation of tabbed area, while default
implementation is NB-specific TabbedContainer tabbed area.

FYI - I'm on vacation this week so I won't be able to integrate changes once they are accepted - saubrecht, rmichalsky
please help maxnitribitt to integrate this API change, I'm completely fine with current version. Thank you.

the changes look fine to me.
if i wanted to be really paranoid, i'd change 

Lookup.getDefault().lookup(TabbedComponentFactory.class).getTabbedComponent(...);

to 

TabbedComponentFactory.getDefault().getTabbedComponent(...);

but it's just nit-picking:)


now to follow the netdev process you have to build core windows and tabcontrol nbms, attach them to this issue and we'll
find somebody from QA to test them.

>I did some final touches to the patch, ensuring that generated javadoc is OK etc. I also removed changes to friend API
of core.windows that were still part of the patch, they shouldn't be needed anymore, but here I'm not sure so please
check it, thanks.

Thanks for your fine tuning. I'm fine with the removed changes to friends API. I used othertabs in lack of a better
name... I guess I'll adopt swingtabs as name for the implementation. 

Created attachment 85764 [details]
added customtabs to public api of tab control

I've updated the patch ( customtabs was missing from public api of tab control ). 

Created attachment 85766 [details]
fixed patch: removed Tabbed from core.windows

Created attachment 85768 [details]
core.windows.nbm and o.n.swing.tabcontrol.nbm

can QA pls assign somebody to do some quick sanity testing of attached nbms?

Hello, I'm back from the vacation and little illness, so let's finish and integrate final patch after testing. I was
told that mmirilovic is on vacation now, so I'll talk to pblaha to assign someone to test attached nbms.

question to maxnitribitt: Will you integrate final patch or do you want me to integrate?

Oops, please ignore my previous post about testing, I was mistaken, blaha and mmirilovic please you can freely ignore
what I wrote, Sun's QA is not planned to test NetDev cases.

According to the NetDev, maxnitribitt should find someone for testing himself. Yes, but this one is only API change and
it does change nothing for standard NB distro. Moreover I already tested and debugged this NetDev case and all was OK.

So from my side: go ahead and please integrate, and thanks for all your work.

Btw, to test whole 150393 issue, some 3rd party L&F and swingtabs/othertabs module should be used for testing, But
that's over the scope of this API change and belongs to 150393.

Hello maxnitribitt, are you experiencing any problems with integration? May I help you somehow? You've been so silent
last two weeks...

I'm reassigning back to you as review is finished and approved.

Hi Dafe, 

sorry for being so silent here and thanks for your offer to help. My plan was to integrate this here or in
comnmunty-main and then go forward to convert my proof of concept implementation into a fully functional one after this
step (mainly debugging and solving an issue with editor panels). I've been discussing this with Jiri offlist and on the
NetDev mailing list. In his opinion this issue needs to be integrated together with
http://www.netbeans.org/issues/show_bug.cgi?id=150393 in the community repository. Since 150393 is not ready for
integration yet in my opinion, I'll have to wait until I'll find a free weekend to fix these issues. --Toni (aka
maxnitribitt)

integrated in community-main. Sorry it took so long. I'm too stupid for mercurial...

:-) Thanks. You are not the only one, we fought quite a few battles with hg too...

Anuradha, it's your turn now. Can you please take build #156 [1] and test that Toni's changes didn't break anything?
Then please add your Go/NoGo verdict here. Thanks. Once you do that I will push the changes to main repository after 6.8
is branched.

[1] http://bertram.netbeans.org/hudson/job/community-main/156

Any word on this?

Anuradha, can you please estimate when you could sanity test Toni's work? Thanks!

I'll contact Toni and will follow up with him

I turned off the community-main and push-to-community-main jobs; push-community-main was already disabled (and had apparently never been run).

I would suggest we just drop the community-main repo; it is mostly merges, and reviewing patches seems more practical anyway. I am attaching diffs of the non-merge changesets.

Created attachment 99150 [details]
All non-merge changes in community-main as of now

Hi!

I'm little bit confused about the state. Started? 

IMHO I can't find any proposed patch in the current sources. @Toni/maxnitribitt: do you like to push this enhancement? 

br, josh.

Josh, the problem is that nobody has tested this contribution to date. Go from community is required to proceed with this and integrate it into the codebase. Would you like to do this instead of Anuradha? Thanks!

(In reply to comment #48)
[...]
> Would you like to do this instead of Anuradha? Thanks!

Ok, I'll test it with the current builds :)

br, josh.

Excellent, once you test the feature thoroughly, please add your comment(s) here and change the status from "Ready to test" to "Tested" in the NetDEV features list:

http://wiki.netbeans.org/NetDEVIssues

Thanks a lot Josh!

Ok, last patch "All non-merge changes in community-main as of now (29.19 KB, patch) 2010-05-18 16:11, Jesse Glick" doesn't apply to the current dev. IMHO Toni (maxnitribitt) needs to create a newer patch. 

Last output lines:

Hunk #1 FAILED at 0
1 out of 1 hunks FAILED -- saving rejects to file core.windows/src/org/netbeans/core/windows/view/ui/Tabbed.java.rej
abort: patch failed to apply
ERROR Command failed:

Command: [hg, import, -v, --repository, xxx\projects\netbeans-main\main, --cwd, xxx\projects\netbeans-main\main, xxx\NetBeansProjects\TabbedPanePatch\tabbedPanePatch.patch]

If I make a mistake, a short note to me. Then I try it again.

br, josh.

(In reply to comment #51)
> Hunk #1 FAILED at 0
> 1 out of 1 hunks FAILED -- saving rejects to file
> core.windows/src/org/netbeans/core/windows/view/ui/Tabbed.java.rej

Probably a conflict with the new license headers in the existing Tabbed.java. Really the original patch was wrong to delete the old class and make a fresh class with the same contents; should rather have used a Mercurial rename. I can try to recreate the diff.

Created attachment 107527 [details]
Updated patch against current trunk

Resolved bogus patch conflict by switching to a rename rather than a copy-and-delete. Did not update license headers in two new files to current Oracle copyright header; not really sure how this works for outside contributions anyway.

Patch is missing API change metadata which would be required before a final commit after passing review - new spec version in manifest.mf of tabcontrol, @since updated accordingly and an apichanges.xml entry for it, usage of new spec version from core.windows.

See http://wiki.netbeans.org/APIReviews for API changes.

any update on this?

Aljoscha, can you please comment on this? If you don't reply by the end of this week (1/20) we will integrate the change. Thanks for your cooperation.

Created attachment 115034 [details]
API changes in o.n.swing.tabcontrol

I have changed Tabbed from interface to abstract class. A Tabbed interface makes for easier implementation but it would also make all future enhancements to tab control backwards incompatible.

Created attachment 115035 [details]
non-API changes in core.window module

(In reply to comment #53)
> Resolved bogus patch conflict by switching to a rename rather than a
> copy-and-delete.

Lost in comment #57 and comment #58 since (a) these are not Mercurial diffs, (b) the file was renamed from core.windows into o.n.swing.tabcontrol. I would recommend using MQ in the future, and never use Team > Export > Export Diff.


[JG01] apichanges.xml entries should be set to <date> of expected integration, not today.


[JG02] supersedes="org.netbeans.core.windows.view.ui.DefaultTabbedComponentFactory" is unnecessary if you specify a position (anything less than MAX_INTEGER in this case).


[JG03] Detailed explanation of how to use TabbedComponentFactory belongs in its Javadoc, not apichanges.xml which need merely mention that it exists.


[JG04] The purpose of Tabbed.Accessor is not clear to me. Why would you not just return Tabbed directly from TabbedComponentFactory? It seems to be used for something else by TopComponentDragSupport but this is undocumented.


[JG05] TabbedSlideAdapter.java seems to have all sorts of changes not clearly related to the API. Same in TabbedAdapter.java. Did you accidentally reformat or something?


[JG06] Enum constants should conventionally be in all caps (VIEW etc.).

Created attachment 115068 [details]
API changes in o.n.swing.tabcontrol

Created attachment 115069 [details]
non-API changes in core.window module

(In reply to comment #59)
> 
> [JG01] apichanges.xml entries should be set to <date> of expected integration,
> not today.
> 
> 
> [JG02]
> supersedes="org.netbeans.core.windows.view.ui.DefaultTabbedComponentFactory" is
> unnecessary if you specify a position (anything less than MAX_INTEGER in this
> case).
fixed in the new patch

> 
> 
> [JG03] Detailed explanation of how to use TabbedComponentFactory belongs in its
> Javadoc, not apichanges.xml which need merely mention that it exists.
fixed in the new patch

> 
> 
> [JG04] The purpose of Tabbed.Accessor is not clear to me. Why would you not
> just return Tabbed directly from TabbedComponentFactory? It seems to be used
> for something else by TopComponentDragSupport but this is undocumented.
fixed in the new patch

> 
> 
> [JG05] TabbedSlideAdapter.java seems to have all sorts of changes not clearly
> related to the API. Same in TabbedAdapter.java. Did you accidentally reformat
> or something?
I changed Tabbed from interface to abstract class and TabbedAdapter extends TabbedContainer so I had to do a lot of refactoring there. It wasn't actually needed for TabbedSlidedAdapter so I've reverted that change.

> 
> 
> [JG06] Enum constants should conventionally be in all caps (VIEW etc.).
fixed in the new patch

TabbedComponentFactory.java seems to have disappeared from the new pair of patches.

Created attachment 115077 [details]
API class missing in the patch

Typo: service=TabbedComponentFactory.class,,position=1000

if there are no more comments, i'll integrate the changes tomorrow

core-main 6c7488cac324

Integrated into 'main-golden', will be available in build *201201260600* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/6c7488cac324
User: S. Aubrecht <saubrecht@netbeans.org>
Log: #169099 - Create TabbedComponentFactory to allow replacing TabbedContainer