I applied this as part of your patch at https://github.com/mwjames/smw-srfgallerycarousel

This looks a bit odd to me though: !$imgTitle->getDBkey() == null

I read that as: if the negation of $imgTitle->getDBkey() is 'sort of equal' to null. Is that what you want it to do?

Please use is_null().

Patch on SRF_Gallery.php

Hope this works, the patch includes:

• Use is_null() instead

and since I'm bit lazy I just put in some other changes related to SRF_Gallery.php

• Tweak css so vertical display is similar to horizontal css
• Use perrow parameter as indicator for the amount of visible and scrollable elements
• Prepare other optional paramters that are not implemented yet
• The jcarousel developer suggested to use wrap=both instead of wrap=circular as default

attachment SRF_Gallary-Carousel-0.1.1.patch ignored as obsolete

SRF_Gallery.php Patch 0.1.2, includes 0.1.1 plus RTL support

This patch includes all above plus

• RTL CSS support

attachment SRF_Gallery-Carousel-0.1.2.patch ignored as obsolete

Can you please use tabs to indent like done in these files already? I fixed the indentation last time, but am not going to keep doing that.

Looks good apart from that.

Fix indentation

OK, this should fix the indentation for all required files including the latest change from r112069

attachment SRF_Gallery-Carousel-0.1.2-v3.patch ignored as obsolete

SRF_Gallery-Carousel-0.1.3.patch for multi instances display

Ah, hope this is the last patch but I finally got the multi-instance display working which means that with 0.1.3 more than one carousel can be displayed per page, the display of a standard gallery combined with gallery carousel works as supposed to be.

All previous changes are included and tested with Chrome and Firefox

Attached:

Great. Applied in r112201, and tested with trunk :)

Some comments though:

• Please provide patches against trunk. If someone makes a change on SVN and you just incorporate it in your patch, merging will result in a conflict since the code is already there.
• You where doing some false selection in the JS :)
• If you have a dictionary of stuff in JS (ie { stuff: ... }), then don't put a comma after the last element, this makes IE cry (yeah, really >_>)
• Avoid aligning assignments in most cases, can be useful in setup files with long lists of similar assignments, but rarely in other places.

One improvement possible in the JS is getting rid of all the var and just assigning directly in the dictionary, like this:

( '#carousel' ).jcarousel( {

parseInt($( '#carousel' ).attr( 'scroll' ) ),
...

Ahhh! Bad timing :)

Can you provide a patch against trunk? :/

Version 0.1.3, patch against r112205

Let's see if we hit the bullseye this time

Attached:

r112215

• Please don't prefix stuff with 'm_' or 'm'
• Code style convention for both JS and PHP is to have spaces between round brackets if there is something in between, so .attr( "id" ) rather then .attr("id")

I made some changes to the PHP (since the merge failed anyway... looking foreward to git :). The style mismatch is really trivial, no need to make another patch for that.

This script allows you to mass fix style issues btw: http://svn.wikimedia.org/viewvc/mediawiki/trunk/tools/code-utils/stylize.php?view=co