<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#ffffff" text="#000000"> 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: <blockquote cite="mid:4D544443.3090201@redhat.com" type="cite">On 02/10/2011 01:13 AM, Endi Sukma Dewata wrote: <blockquote type="cite">On 2/9/2011 7:06 PM, Adam Young wrote: <blockquote type="cite"> </blockquote> 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. </blockquote> I had mIssed the classes that these things needed. Added them back in. <blockquote type="cite"> 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'); </blockquote> Done. Good idea/ <blockquote type="cite"> 3. The indentation of the target_types array in aci.js is inconsistent. </blockquote> Fixed <blockquote type="cite"> 4. The IPA.hidden_widget doesn't seem to be used. Should this be removed? </blockquote> Gone baby gone <blockquote type="cite"> 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. </blockquote> Removed <blockquote type="cite"> 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. </blockquote> Agreed. This was actually part of trial and error to get it to work, and it didn't need to be there. Gone. <blockquote type="cite"> 7. There are some jslint warnings. </blockquote> Fixed <pre wrap=""> <fieldset class="mimeAttachmentHeader"></fieldset> _______________________________________________ Freeipa-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> </body> </html>