Page MenuHomePhabricator

Implement collapsible tables in core javascript
Closed, ResolvedPublic

Description

Author: happy_melon

Description:
Against r47169, implements CollapsibleTables from en.wiki in wikibits.js

This functionality is very much taken for granted, we forget that it's not actually available in core, and must be ported to every wiki that wishes to use it (which is a large number). I've rewritten the code from en.wiki to remove the external dependencies, it should now run on a clean MW installation using only the getElementsByClassName() function that's already in wikibits.js. The show/hide link titles are translatable (they use the same values by default as the show/hide links in the expanded watchlist, although of course the JS variables can be overwritten for more specific customisation).

Only thing that I'm not sure about is how pages will appear when this code is deployed if a wiki already has an implementation in their Common.js. I suspect double show/hide links will appear; but since AFAIK core JS executes before site JS before user JS, there's nothing that can be added to the core code to prevent the site code from acting (short of removing the "collapsible" classes, which might be a *little* extreme :D).

Patch needs thorough review (I've tested the JavaScript, but not the implementation).


Version: 1.15.x
Severity: enhancement

Attached:

Details

Reference
bz17456

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:26 PM
bzimport set Reference to bz17456.
bzimport added a subscriber: Unknown Object (MLST).

What's this variable intended to mean? It seems a little clear and there's no way to configure it, which would appear to make making it a variable kind of silly. :)

+ $vars['wgCTAutoCollapse'] = '2';

happy_melon wrote:

It's the 'autocollapse' threshold: set it to 'n', and then if there are more than n collapsible tables on a page, the nth table onwards will be collapsed by default if it has the "autocollapse" class. Perhaps it would be cleaner to declare it as a pseudo-default:

if( autoCollapse = typeof(wgCTAutoCollapse) != 'undefined' ? wgCTAutoCollapse : 2;
....

Er, why would there be any need for a threshold here?

mike.lifeguard+bugs wrote:

(In reply to comment #3)

Er, why would there be any need for a threshold here?

I think the intent is that if you have one or two that's not a problem, but if you have 100 of them, it'll cause lots of unwanted clutter, so collapse everything after #2 (but IIRC there is still some way to specify "No, I /really do/ want the 100 tables to be uncollapsed - don't apply that limit please" if you need it).

happy_melon wrote:

And yet, if you were doing that, you just wouldn't use the 'autocollapse' class... Now you come to mention it, it seems decreasingly useful: I just copied it from the en.wiki JavaScript. It would be easy for sites to write extra 'modules' that collapse things according to other parameters - en.wiki will need to do that anyway, having the "innercollapse" and "outercollapse" classes to play with. Perhaps it's only basic "collapsed" that should be supported in core...

herd wrote:

I would suggest just adding simple helper functions to core, or doing this all as a fully integrated (with non-JS and print gracefully handled).

Such helper functions could simply be, for example, easy ways to create a show/hide link and insert it into a specific node, and associate it with a given sibling object to that node. The TOC show/hide functionality could even be abstracted and used as the target of such aforementioned-helperfully-generated (is that a word?) links.

But, adding yet another rarely-used over-specific chunk of js bloat to core is a /bad idea/, IMHO.

happy_melon wrote:

"Rarely used"?? This code, or the old NavFrame code that it replaces, is included almost verbatim on seventeen of the top twenty Wikimedia wikis, including all ten largest wikipedias, and on the majority of the external wikis I checked, including Conservapedia, Uncyclopedia and Wikia. I suspect that your comment may have been something of a knee-jerk reaction. Equally, I don't see how this is possible to "do this all as a fully integrated"[sic] without JavaScript: it is already designed to fail gracefully in non-JS; successful print handling is as simple as adding

@media print {

table.collapsible td,
table.collapsible th {  /*For IE*/
    display:inline !important;
}
table.collapsible td,
table.collapsible th {  /*For other browsers*/
    display:table-cell !important;
}

}

Along with the other CSS declaration - how exactly would you improve this further? By contrast, restricting the core stuff to helper functions achieves absolutely nothing: such features are the *easy* parts of this script to code, and wrapping them in another layer of abstraction would probably make the script *more* difficult to implement, not less. I agree that the collapsible toc could be consolidated into this feature, but given that the ToC is a table already, if this feature was implemented fully it would be utterly trivial to simply give the ToC the "collapsible" class and let the JS take over; we can lose the existing ToC code altogether if we incorporate the cookie 'memory' into CollapsibleTables, although I'm not sure how best that could be done. So really I don't see the strengths in your argument.

mike.lifeguard+bugs wrote:

(In reply to comment #7)

"Rarely used"?? This code, or the old NavFrame code that it replaces, is
included almost verbatim on seventeen of the top twenty Wikimedia wikis,
including all ten largest wikipedias, and on the majority of the external wikis
I checked, including Conservapedia, Uncyclopedia and Wikia.

I think that comment was meant more in the sense of the number of pages that use it. Collapsible tables just aren't that widely used.

happy_melon wrote:

I'm willing to bet money that they're used on more pages than the SortableTables code that comprises a full *third* of wikibits.js. What's the difference?

Roan said we can handle this with about ~5 lines of JQuery now that we can package it.....

(In reply to comment #10)

Roan said we can handle this with about ~5 lines of JQuery now that we can
package it.....

I was talking about show/hide for infoboxes and the like, not tables necessarily.

thedj has a jQuery implementation at http://commons.wikimedia.org/wiki/MediaWiki:CollapsibleTemplates.js (not sure it's the same feature, no idea how stable/mature it is), and Krinkle is working on porting general core JS functionality to the 21st century (i.e. using jQuery and ResourceLoader).

(In reply to comment #11)

thedj has a jQuery implementation at
http://commons.wikimedia.org/wiki/MediaWiki:CollapsibleTemplates.js

According to its history, that's mostly DieButch's work; TheDJ hasn't ever touched that page (though I'm not saying the code wasn't based off of earlier work by TheDJ; that wouldn't surprise me).

i handed the link to Roan over IRC. tad of confusion :D

Aah, makes sense (also, posting to note I messed up the username too - it's DieBuche).

Aah, makes sense (also, posting to note I messed up the username too - it's DieBuche).

happy.melon.wiki wrote:

Implemented in r78865.

happy.melon.wiki wrote:

Better implementation via jQuery plugins in r78914.