Page MenuHomePhabricator

Improvements to bookmanager.js
Closed, DeclinedPublic

Description

Copying from r84906#code-comments,

Since the script at bookmanager.js interacts with the DOM, it should be executed only when document is ready, as in:
$(function() {
//Code goes here...
});

Besides, I think you should replace
if ( $prev[0] ) by if ($prev.length)
if ( $next[0] ) by if ($next.length)

And if would be good idea to add an id for the top navigation and another for the botton, so that $( ' .mw-book-navigation ' ) can be replaced by something like
$( '#some-id-for-top, #some-id-for-bottom' ).find( ' .mw-book-navigation ' )
, which is more efficient. See
[[mw:JavaScript_performance#Selector_performance_.28jQuery.29]]


Version: unspecified
Severity: normal

Details

Reference
bz30998

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:52 PM
bzimport set Reference to bz30998.

It seems that the first part is already fixed on trunk:
http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/BookManager/client/bookmanager.js?revision=87713&view=markup

I think currently the only thing needed is to use ids for the top/bottom navigation bars.

Rephrasing bug summary. Jumped out in general searches and thread views, mentioning book manager now.

Aklapper lowered the priority of this task from Medium to Lowest.Jan 2 2015, 1:30 PM
MarcoAurelio subscribed.

Closing all tasks as the BookManager extension is being archived as per T180020: Archive the BookManager extension and its successor, BookManagerv2 being archived as well.