Additional to setting simple parameters e.g. height: 300, Advanced Options should be able to handle functions e.g. before: function(curr, next, opts, fwd) { ... .

In the case where a user-defined parameter will overwrite a views_slideshow parameter, the code creates a new function to call both old and new functions.

CommentFileSizeAuthor
#4 eval-functions_0.patch887 bytesredndahead
#2 parse_function_closure.patch2.74 KBdingbats
parse_function.patch2.45 KBdingbats

Comments

ceardach’s picture

ceardach commented

This patch allowed me to enter the following:

  before: function(curr, next, opts) { 
    if (curr == next) { 
      opts.$cont.css({ 
        height: next.cycleH + 'px' 
      }) 
    } else { 
      opts.$cont.animate({ 
        height: next.cycleH + 'px' 
      }, opts.speed); 
    }
  }

The code snippet resizes the container at every transition and allowed me to avoid some issues I was having with my usage of views slideshow.

Since it will be difficult to predict and account for all use cases of slideshow behaviour, this patch would provide added flexibility. I recommend it being committed.

dingbats’s picture

dingbats commented
StatusFileSize
new parse_function_closure.patch2.74 KB

The previous patch failed to call the function with more than one Advanced Option. The new patch resolves this issue by wrapping the functions inside a closure. I've also simplified the regex.

redndahead’s picture

redndahead commented
Priority: Normal » Critical

Marking as critical to look at before release.

redndahead’s picture

redndahead commented
StatusFileSize
new eval-functions_0.patch887 bytes

Adding the patch from #733328: Advanced Settings does not honor functions by jennycita

Even though this one is newer marking the other as duplicate. I would like it if dingbats or ceardach could test this patch as it's a simpler fix. Any possible issues?

jennycita’s picture

jennycita commented

Subscribing. My patch was quick-and-dirty and requires the entire function to be on one line -- a substantial improvement over not being able to use functions at all, but an approach that allows multi-line function overrides is worth looking into.

redndahead’s picture

redndahead commented

@jennycita thanks that's good to know.

dingbats’s picture

dingbats commented

@redndahead My patch is a little more complicated but with two advantages: 1) it doesn't use eval, and 2) it doesn't overwrite previously set functions.

To illustrate 2): have a look at lines 16-45 where settings.opts is defined. You'll see that before and after already have functions, which jennycita's patch will allow Views administrators to overwrite consequently breaking the slideshow. My patch creates a closure in which the previous function and the new function are called.

It may also be worth noting that the ability to parse multi-line functions is a separate issue and would require a separate patch to change how advanced options are tokenized; currently they are split by newlines on line 58: var advanced = settings.advanced.split("\n");.

redndahead’s picture

redndahead commented

Thanks for the update. I'm not sure how to tokenize any better without making it somewhat look clunky. Maybe with some javascript I can do something that won't look completely hideous....maybe

For now this may have to be limited to keeping it all on one line.

jennycita’s picture

jennycita commented

Ah, misunderstood -- Comment #1 lead me to believe this solution accepted multi-line functions. Looking at the patch, I see that's not the case.

redndahead’s picture

redndahead commented
Status: Needs review » Fixed

This has been committed. Thanks

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.