quicksettings

quicksettings copied to clipboard

Hi Keith,

Let me start by saying I really love QuickSettings, much more so than dat.GUI. You have done amazing work! This is why I feel like I should contribute.

However, there is one thing that keeps bugging me. It is the fact that when you call getValue() on a dropdown, it returns an object instead of the actual value. This keeps tripping me up. I understand that you want to provide a way to retrieve the index but, to me, this feels inconsistent with the rest of the API and I seldom need to use the index anyway...

What if you provided a getIndex() function on dropdown's or perhaps a getDetails() function which would return an object with index, label and value properties. This would allow the getValue() function to behave in a standard way accross the API while allowing the possibility to retrieve additional details.

I understand this change would break backwards compatibility (which is never desirable) but I believe it would result in a cleaner API and less confusion from developers.

Perhaps you have already considered this and decided against it but, if not, I would invite you to think about it.

Again, thanks for the great work.

I appreciate the feedback. Gotta think about this one a bit.

Maybe a little off-topic, but how to setValue of a dropdown control?

@linux-man It works the same way as other controls. Are you having an issue?

Nothing but laziness - it was late in the evening. I examined the code. Dropdown accept an Object or an index. Since the most used information is the "label" (I guess), you could maybe accept it. I propose something like:

setValue: function(value) {
          var index;
          var options = this.control.options;
          if(typeof value == 'string') {
            for(var n = 0; n < options.length; n++) if(options[n].label == value) index = n;
          }
          else if(value.index != null) {
              index = value.index;
          }
          else {
              index = value;
          }

I agree that a getIndex() and getValue() (or preferably a getLabel()) using the label would avoid some unnecessary management of the object you are using now.

Still need to dig into this one. I'm sure it can be improved.