Added "optionsFunction" to config

evilham commented 11 years ago

Instead of saving the options as array, completely will run the function given in the optionsFunction attribute of the config object and use the output array as the options for this textInput.

A bit of code to illustrate why this may be wanted:

<html>
<head>
  <script src='/js/complete.ly.1.0.1.js'></script>
</head>
<body>
<div id='div1' style='border:1px solid #999;'></div>
<div id='div2' style='border:1px solid #999;'></div>
<script>
function options() {
  // default values
  var ops = ['cocoa','coffee','orange'];
  // do some logic and change to alcoholic beverages.
  if (true)
    ops = ['vodka', 'whiskey', 'rum'];
  return ops;
}
var config = {
        fontSize: '24px',
        fontFamily: 'Arial',
        color: '#933',
        'optionsFunction': options,
   };
completely(document.getElementById('div1'), config);
completely(document.getElementById('div2'), config);
</script>
</body>
</html>

evilham commented 11 years ago

Just realised that the dropdown won't work because of line 270. Should be:

dropDownController.refresh(token, options);

instead of:

dropDownController.refresh(token, rs.options);

This is my commit including this fix: https://github.com/evilham/completely/commit/242b3ada0e17535adecf2968cec5112c8cb56f8f

lorenzoongithub commented 11 years ago

@evilham Thanks for your suggestion to add an optionsFunction. I might be missing something here.. Why would it be required ? and what would invoke it ?

evilham commented 11 years ago

I picture it as a really light function that probably does a couple ifs and returns the array, depending on the state of something on the site or some extra infos queried via an HttpRequest or... The function ought to be "light", as it is called once whenever the repaint method runs.

This approach shines though when you make a form with several inputs that are to have the same Options and it is possible that they change; in such a case you can just change the output of the optionsFunction without needing to iterate through all of the Completely objects and setting their .option property.

lorenzoongithub commented 11 years ago

@evilham So,if understand you correctly this function would be called on 'repaint' and you would use it to change the option array. In that case, you can do it already: you could redefine repaint so to call the original function you are overriding (the original repaint) and then reassign the option array as you wish or call your optionMethod() I honestly welcome feedback and change requests but I have to confess that I am not convinved this one is worth. Besides, it'd break the original API (since options is an array) and people who are using another version would have to have a roadmap to upgrade. I will think about it but for now I will not act on this

evilham commented 11 years ago

I hate to ask, but did you check the code in the commits? It's just 7 lines long and is probably more understandable than words.

This doesn't change the existing API, you are very right to say that would be a very bad thing. It just adds an extra way for developers to define the options to be used on a Completely object.

The way it works is as follows:

If the optionsFunction parameter was given in the config object, the local variable options inside the repaint method will be defined as the output of the optionsFunction.
If the parameter was not given, the local variable options inside the repaint method will be defined as the options attribute of rs.
Use the local variable options to do the magic of Completely.

Notice that the way Completely works is only affected if the optionsFunction parameter was given in the config.

While it is true that one could simply redefine the repaint method and set the options property there, I think that would be a much more intrusive approach; it also requires that the user knows the internals of Completely and they keep a reference to the Completely objects (plus, the options are saved in every single Completely object).

I think in general, users are better off just setting .options, but if one has a set of inputs that are to have the same options and these options are either very large or prone to change, the optionsFunction approach offers a quick way to deal with that.

I created a JSFiddle so you can see the potential of the optionsFunction approach: http://jsfiddle.net/QGT8P/8/ This was a simple example where one adds new options to the Completely inputs, but it can easily get more sophisticated with triggers or external sources.

Hopefully you'll reconsider merging the commits, since they are really unintrusive and provide something that may be handy for developers with deadlines.

lorenzoongithub commented 11 years ago

I will think more about it. Thanks a lot for your work.

lorenzoongithub / completely

Added "optionsFunction" to config #4