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

[Adam Young]

On 09/30/2010 09:20 AM, Adam Young wrote:
> On 09/30/2010 09:18 AM, Adam Young wrote:
>> 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
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
ACKed in IRC, pushed to master
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20100930/f4d29cb2/attachment.htm>