[Freeipa-devel] admiyo-freeipa-0048-Item-Level-Undo.patch

[Adam Young]

On 09/29/2010 11:07 PM, Endi Sukma Dewata wrote:
> ----- "Adam Young"<ayoung at redhat.com>  wrote:
>
>    
>> Should have remembered this approach, standard JS way to deal with
>> undefined values.
>>      
>    
> admiyo-freeipa-0048-3-Item-Level-Undo.patch
>
> A few notes:
>
> 1. You're replying to the wrong thread :)
>    

Fixed :)

> 2. The undo button will only appear when the input field loses focus. Ideally
> it should appear as soon as the value is changed, but I'm not sure if it's
> possible to do that in JS. This can be addressed in the future.
>    

Right.  I was scared off by the docs that claimedthings were so 
different between browsers for the keydown and keypress events, but it 
looks like whatever differences there are are irrelevant here.  Change 
to triggering on keydown.

> 3. The hint_span doesn't seem to be used consistently in details.js:272-297:
>
> ipa_insert_first_dd(
>      jobj, ipa_create_input(obj_name, attr, value[0],hint_span)
> );
> ipa_insert_other_dd(
>      jobj, ipa_create_input(obj_name, attr, value[i],hint_span)
> );
> ipa_insert_other_dd(
>      jobj.next(), _ipa_a_add_template.replace('A', attr)
> );
> ipa_insert_first_dd(
>      jobj, _ipa_a_add_template.replace('A', attr) /*.append( hint_span)*/
> );
> ipa_insert_first_dd(
>      jobj, ipa_create_input(obj_name, attr, '')/*.append( hint_span)*/
> );
>    

Yeah, but that mirrors the original code.  I think figuring out where to 
put the hintspan and how to trigger it is a different patch.  Hintspan 
should probably not be duplicated like this at all.

> 4. I think the statement on line 341 should be removed because it redefines
>     the input variable:
>
>          var input = $("<label>",{html:value.toString()});
>
> 5. There is a trailing whitespace on line 337.
>    
Why, so there is....was.
> Thanks!
>
> --
> Endi S. Dewata
>