[SRF] 1.7 [patch]: SRF_Gallery.php introduce new parameter galleryformat for jcarousel plug-in
Closed, ResolvedPublic

Description

SRF_Gallery.php patch

SRF_Gallery.php produced the following error:

Notice: Undefined property: StubObject::$mOutput in ...\extensions\SemanticResultFormats\Gallery\SRF_Gallery.php on line 166

Fatal error: Call to a member function addImage() on a non-object in ...\extensions\SemanticResultFormats\Gallery\SRF_Gallery.php on line 166

Patch tested on:
Semantic Result Formats (Version 1.7); Semantic MediaWiki (Version 1.7.0.2); MediaWiki 1.18.0; PHP 5.3.8; MySQL 5.5.16


Version: unspecified
Severity: normal

attachment SRF_Gallery.patch ignored as obsolete

bzimport set Reference to bz34411.
mwjames created this task.Via LegacyFeb 15 2012, 7:36 AM
JeroenDeDauw added a comment.Via ConduitFeb 20 2012, 6:53 PM

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().

Unknown Object (User) added a comment.Via ConduitFeb 21 2012, 10:57 AM

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

Unknown Object (User) added a comment.Via ConduitFeb 21 2012, 12:34 PM

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

JeroenDeDauw added a comment.Via ConduitFeb 21 2012, 10:03 PM

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.

Unknown Object (User) added a comment.Via ConduitFeb 22 2012, 7:34 AM

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

Unknown Object (User) added a comment.Via ConduitFeb 23 2012, 1:15 PM

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: SRF_Gallery-Carousel-0.1.3.patch

JeroenDeDauw added a comment.Via ConduitFeb 23 2012, 1:21 PM

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' ) ),
...
JeroenDeDauw added a comment.Via ConduitFeb 23 2012, 1:22 PM

Ahhh! Bad timing :)

Can you provide a patch against trunk? :/

Unknown Object (User) added a comment.Via ConduitFeb 23 2012, 2:34 PM

Version 0.1.3, patch against r112205

Let's see if we hit the bullseye this time

Attached: SRF_Gallery-Carousel-0.1.3-v2.patch

JeroenDeDauw added a comment.Via ConduitFeb 23 2012, 4:33 PM

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

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.