[Freeipa-devel] [PATCH] 32-47 #2040, #1515 Refactor UI widgets

[Endi Sukma Dewata]

On 12/1/2011 10:45 AM, Adam Young wrote:
> On 12/01/2011 10:02 AM, Petr Vobornik wrote:
>> 1) Should widget expect that array of values like ['value'] will be
>> always passed to update(values) method or the update method should
>> also work with simple values like 'value'?
>
> The widget tests are higly subjective. Do not be afraid to change them,
> especially if the assumptions they are written against have changed.
> They were written more as a way to exercise the widgets than a
> codification of behavior.

The best probably would be to support both, but the code would have to 
check the param type, which would make it more complex. If we're just 
going accept 1 type we should take the array because it can accommodate 
single value too. Some single-valued attributes are returned as arrays too.

>> 2) Should widget load its own metadata - based on entity and its name
>> or it is responsibility of field (current implementation)?
>
> Personally, I'd prefer if the Metadata was passed to the constructor, as
> that allows us to compose it in multiple ways. That will be useful in
> the future. INfact, I would prefer it if, instead of the Metatdata as a
> chunk, we passed the indivual pieces that are required (pattern
> specifically) so that how to build the object can differ, but I wanted
> to hold off on doing anything like this until we had at least one more
> specific project to integrate with. GO with the most striaght forward
> solution for now.

Yeah, the metadata should be supplied by the field builder into the 
field's constructor. Then the field can use it to set the labels in the 
widget.

>> 3) Should custom dirty checks, or other boolean checks (validate)
>> always return pure boolean value, or is integer also acceptable. Usage
>> of &= or |= operators is producing integer values because it actually
>> means 'foo = foo & bar' not 'foo = foo && bar'
>
> Is there really a difference in Javascript? I so long as they evaluate
> to true/false, it is probably fine, but is there the possibility for
> confusion?

I think we should make sure we use boolean operator instead of bitwise. 
I'm not sure if we're actually affected, but there's a difference:

  1 && 2 => true
  1 & 2 => 0

>> Also all jsl warnings are fixed and jsl.conf for tests is added.

There's a whitespace warning in patch #49.

-- 
Endi S. Dewata