[Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons

[Adam Young]

Last version was a little too zealos in remivng style info, and I 
removed the code that hid the select boxthat chose the target.  Added 
that code back in here.


On 02/10/2011 03:02 PM, Adam Young wrote:
> On 02/10/2011 01:13 AM, Endi Sukma Dewata wrote:
>> On 2/9/2011 7:06 PM, Adam Young wrote:
>>>
>>
>> A few comments:
>>
>> 1. The functionality seems to be working, but the layout is a bit 
>> different. Previously the label (e.g. Filter) and the widget (e.g. 
>> text field) occupy the same line. Right now they occupy different 
>> lines and not aligned with the labels & widgets above it (e.g. 
>> Permission name). I'd like the UXD team to review this change.
>
> I had mIssed the classes that these things needed.  Added them back in.
>
>>
>> 2. The jQuery selectors on lines 427, 462, 472 in aci.js are not 
>> qualified, so they will be doing a global search. I'd rather store 
>> the object reference somewhere and use it directly without searching 
>> for it again. For example, line 411 can be changed as follows:
>>
>>   target_type.container = $('<dl/>', {
>>
>> Then line 427 can be changed as follows:
>>
>>   target_type.container.css('display', 'block');
>
> Done.  Good idea/
>
>>
>> 3. The indentation of the target_types array in aci.js is inconsistent.
> Fixed
>>
>> 4. The IPA.hidden_widget doesn't seem to be used. Should this be 
>> removed?
> Gone baby gone
>>
>> 5. For the changes in dialog.js, it's not necessary to check 
>> section.reset()'s presence before calling it. All sections will have 
>> a reset() function because it's inherited from the base class.
>
> Removed
>>
>> 6. For the changes in widget.js, let's do this in a separate patch. 
>> We'll combine the create/setup in a more consistent way.
>
> Agreed.  This was actually part of trial and error to get it to work, 
> and it didn't need to be there.  Gone.
>>
>> 7. There are some jslint warnings.
>>
> Fixed
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110210/c3055151/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-admiyo-0191-5-target-section-without-radio-buttons.patch
Type: text/x-patch
Size: 25813 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110210/c3055151/attachment.bin>