@Ladsgroup Could you help us with code review and enabling of this Gadget on MediaWiki.org? I'm guessing that after the code review step we share about the Gadget on the Project:Current_issues page to get community consensus...

In T228920#5363084, @srishakatux wrote:

@Ladsgroup Could you help us with code review and enabling of this Gadget on MediaWiki.org? I'm guessing that after the code review step we share about the Gadget on the Project:Current_issues page to get community consensus...

Okay, I tried to copy the code in my console in mediawiki.org (in API:Pageprops page) and it didn't add anything. By looking at the code, it seems it makes the tabs then add them to the page using $( this ).empty().append( panelLayout.$element ); which to me seems a little bit too loose (Put a better selector on it).

Another issue I found is the performance of the gadget, It should avoid loading ooui, specially on page views. It should lazy load them when someone clicks on the tabs

In T228920#5371149, @Ladsgroup wrote:

it didn't add anything.

It is as expected because we are using the <div class="mw-gadget-tabbedwindow"> to wrap the === PHP ===, === Python === etc. API:Pageprops does not have any sample code nor it is wraped in mw-gadget-tabbedwindow. Please look once the usage at Template:TabbedWindowBegin and API:Sandbox.

which to me seems a little bit too loose (Put a better selector on it).

As we are using div, So it is safe to empty and append in it. because OOUI creates its own DOM instead of changing the current elements.

It should avoid loading ooui, specially on page views.

This point, I already discussed with @srishakatux. But the good thing is that we have a limited number of pages in API namespace (70-100).

Also, we supportedNamespaces variable to check that page is belong to the API namespace or not. Should I change to poistion of the mw.loader.using to execute after supportedNamespaces check? like that

$.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core', 'mediawiki.Title' ), $.ready ).then( function () {

    .
    .
    .


    if ( action !== 'view' ||
        namespace.indexOf( supportedNamespaces ) === -1 ||
        isTalkPage === true ||
        tabwindow === 0 ) {
        return;
    } else {
        makeTabWindow();
    }
} );

to

$.when( mw.loader.using( 'mediawiki.Title' ), $.ready ).then( function () {

    .
    .
    .

    if ( action !== 'view' ||
        namespace.indexOf( supportedNamespaces ) === -1 ||
        isTalkPage === true ||
        tabwindow === 0 ) {
        return;
    } else {
        $.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core' ), $.ready ).then( function () {
            makeTabWindow();
        )};
    }
} );

In T228920#5371307, @Jayprakash12345 wrote:

This point, I already discussed with @srishakatux. But the good thing is that we have a limited number of pages in API namespace (70-100).

It seems okay to me.

Also, we supportedNamespaces variable to check that page is belong to the API namespace or not. Should I change to poistion of the mw.loader.using to execute after supportedNamespaces check? like that

Yes, the former, actually makes ooui loaded on every page of mediawiki.org unconditionally, which is a very heavy module. (Is this happening on test.wikipedia.org? Please fix that too).

I think your template should just add a class like tabbedCodeSample and then the gadget hook into that (instead of checking namespace, etc.) like:

if ( action === 'view'  && $('.tabbedCodeSample').length !== 0 ) {
   $.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core' ), $.ready ).then( function () {
            makeTabWindow();
        )};
}

That would inject and load the modules exactly where needed.

In T228920#5371379, @Ladsgroup wrote:

I think your template should just add a class like tabbedCodeSample and then the gadget hook into that (instead of checking namespace, etc.) like:

if ( action === 'view'  && $('.tabbedCodeSample').length !== 0 ) {
   $.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core' ), $.ready ).then( function () {
            makeTabWindow();
        )};
}

That would inject and load the modules exactly where needed.

We are adding mw-gadget-tabbedwindow class by Template. So that it can't conflict with others. We have tabwindow variable in javascript that checks the length of the div tag having mw-gadget-tabbedwindow class. I am sure the makeTabWindow(); executing only when they found tag length, action view, and not talk page.

The problem was to load OOUI unconditionally on every page. I have amended the script here . and test in local enveriment. Now OOUI will load when there will be any div.mw-gadget-tabbedwindow on the page. Now someone needs to amend the Testwiki script.

Thanks, @Ladsgroup for your helpful feedback. I am not sure what the implications on performance would be, but I am wondering if for Gadgets we recommend adding dependencies in the Gadget definition itself, then would it matter at what point in the code we use $.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core' ), $.ready ).then( function () {... and do we even have to load modules via mw.loader.using in the script?

I've updated the script on test wiki with the changes @Jayprakash12345 has made. https://test.wikipedia.org/wiki/MediaWiki:Gadget-tabbedwindow.js.

@Ladsgroup Could you maybe take another look and suggest next step in the process? :)

In T228920#5374448, @srishakatux wrote:

Thanks, @Ladsgroup for your helpful feedback. I am not sure what the implications on performance would be, but I am wondering if for Gadgets we recommend adding dependencies in the Gadget definition itself, then would it matter at what point in the code we use $.when( mw.loader.using( 'oojs-ui', 'oojs-ui-core' ), $.ready ).then( function () {... and do we even have to load modules via mw.loader.using in the script?

The new version got it fixed now but before that it was loading a very heavy JS module in every page view, all pages. This is fixed now.

The code has my +2 to be in mediawiki.org. Now should I move it mediawik.org and enable it for some and then later and after consensus we enable it for everyone?

@Ladsgroup Yes, please! Go ahead and enable it for some on MediaWiki.org. Is it okay if @Jayprakash12345 creates a topic on https://www.mediawiki.org/wiki/Project:Current_issues and seek any feedback or concern on enabling the Gadget for everyone??

I added the gadget, you can enable it now.

The gadget has deployed by default during Wikimania 2019 by @Ladsgroup.

Thank you Amir and nice to meet you :)