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

[Adam Young]

On 02/10/2011 03:09 PM, Adam Young wrote:
> 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
>
>
> _______________________________________________
> 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/2c7e7d32/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-admiyo-0191-6-target-section-without-radio-buttons.patch
Type: text/x-patch
Size: 25701 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110210/2c7e7d32/attachment.bin>