Page MenuHomePhabricator

Change of way to initialize editToolbarConfiguration structure
Closed, ResolvedPublic

Description

Currently, you are using var editToolbarConfiguration = { ... to initialize it, and do it after user&site js are loaded. I hope that you can change to use jQuery.extend and load it before loading of user&site js. This will enable us to customize it easier.


Version: unspecified
Severity: normal

Details

Reference
bz20125

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:53 PM
bzimport set Reference to bz20125.

It *is* easy to customize it:

addOnloadHook(function() {

if ( typeof editToolbarConfiguration != 'undefined' ) {
        // Do stuff
}

});

Resolving as WORKSFORME, please REOPEN if there's a compelling reason why EditToolbar.js has to be before user/site JS.

I think addOnloadHook is just something like a hack. It's available only because toolbar's initialization hooks $(document).ready.

What's the problem if the order is changed?

(In reply to comment #2)

I think addOnloadHook is just something like a hack. It's available only
because toolbar's initialization hooks $(document).ready.

What's the problem if the order is changed?

The core code doesn't facilitate adding extension JS before user/site JS. I've fixed the issue in a different way in r54646: user/site JS can now customize the toolbar with:

$(document).bind('toolbarConfig', function() {

// Do stuff with editToolbarConfiguration

});

(wrapped in a check for $ being defined, of course).

Will it be better if editToolbarConfiguration object is passed into the event handler? (but I don't know where it can be put)

This can avoid hard coding of variable name editToolbarConfiguration.

(In reply to comment #4)

Will it be better if editToolbarConfiguration object is passed into the event
handler? (but I don't know where it can be put)

You'll want to change in, and you can't pass stuff by reference in JS AFAIK.

javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);

This says 1.

(In reply to comment #7)

javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);

This says 1.

Holy crap, that actually works :O Added parameter in r54711. Usage:

$(document).bind('toolbarConfig', function(event, config) {

// config is the editToolbarConfiguration object
// Note that it's the 2nd parameter

});

(In reply to comment #8)

(In reply to comment #7)

javascript:function c(x){x.a=1;}var o={};c(o);alert(o.a);

This says 1.

Holy crap, that actually works :O Added parameter in r54711. Usage:

$(document).bind('toolbarConfig', function(event, config) {

// config is the editToolbarConfiguration object
// Note that it's the 2nd parameter

});

Please note that a different API for adding, modifying and removing toolbar controls is underway and the above will soon be obsolete. Apologies for any inconvenience caused.

when will a stable api be available?

(In reply to comment #10)

when will a stable api be available?

When it's done :D

Rest assured that as long as a stable API is not available, the original hook will not go live on Wikipedia (in fact, we intend to remove it when we have the new API, so that hook will not go live on WP ever).

(In reply to comment #11)

(In reply to comment #10)

when will a stable api be available?

When it's done :D

Rest assured that as long as a stable API is not available, the original hook
will not go live on Wikipedia (in fact, we intend to remove it when we have the
new API, so that hook will not go live on WP ever).

And it's there. Quote from bug 20134 comment #6 (where all this stuff was mis-posted, my apologies):

This is a good example of the API in use. (it's our tests to make sure it's
working)

http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/UsabilityInitiative/js/tests/wikiEditor.toolbar.js?view=markup

Why not follow some patterns like method invocation in jQuery UI?

$('#wpTextbox1').wikiEditor('add', { /* options */ });

and some more...

$('#wpTextbox1').wikiEditor('get', { index: 0 }).bind('x-click', function(event) { // avoid browser's click event

return { /* what to do */ };

});

$('#wpTextbox1').bind('wikiEditorClick', function(event, button) {

return { /* what to do */ };

});

(In reply to comment #13)

Why not follow some patterns like method invocation in jQuery UI?

$('#wpTextbox1').wikiEditor('add', { /* options */ });

and some more...

$('#wpTextbox1').wikiEditor('get', { index: 0 }).bind('x-click',
function(event) { // avoid browser's click event

return { /* what to do */ };

});

$('#wpTextbox1').bind('wikiEditorClick', function(event, button) {

return { /* what to do */ };

});

I don't know why it wasn't done here, but I am going to use there patterns in my rewrite of mwsuggest.js to a more generic suggestions jQuery plugin.