[Freeipa-devel] [PATCH] Multicolumn enrollment dialog

[Adam Young]

On 12/02/2010 10:19 AM, Adam Young wrote:
> On 12/01/2010 08:56 PM, Endi Sukma Dewata wrote:
>> Hi,
>>
>> Please review the attached patch. Thanks!
>>
>> https://fedorahosted.org/reviewboard/r/112/
>>
>> The enrollment dialog has been modified to use scrollable tables that
>> supports multiple columns to display the search results and selected
>> entries. The columns are specified by calling create_adder_column()
>> on the association facet. By default the tables will use only one
>> column which is to display the primary keys.
>>
>> The following enrollment dialogs have been modified to use multiple
>> columns:
>>  - Group's member_user
>>  - Service's managedby_host
>>  - HBAC Service Group's member_hbacsvc
>>  - SUDO Command Group's member_sudocmd
>>
>> The ipa_association_table_widget's add() and remove() have been moved
>> into ipa_association_facet so they can be customized by facet's
>> subclass. The ipa_table's add_row() has been renamed to add_record().
>>
>> Some old code has been removed from ipa_facet_create_action_panel().
>> The code was used to generate association links from a single facet.
>> It's no longer needed because now each association has its own facet.
>>
>> The test data has been updated. The IPA.nested_tabs() has been fixed
>> to return the entity itself if IPA.tab_set is not defined. This is
>> needed to pass unit test.
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
> Looks good.  Some nits.
>
> Move the "width: 200px" into the style sheet.   We should have a css 
> class  that is used for all of the checkbox columns.
>
> Why are the is create_adder_column on the association facet and not 
> the adder object?  Shouldn't it be adder.add_column?
> Remove the parentesis in these and just ue the plural.  var 
> header_message = that.other_entity + '(s) enrolled in '  +  
> that.entity_name + ' ' + that.pkey;
>
> That string actually needs to come from the association definition.  I 
> realize that these are autogenerated, but the generic word "enrolled" 
> doesn't work for the majority of the associations.  For instance, user 
> should say:  "groups containing user kfrog".  You can use the plural 
> name of the object out of the meta data for the other entity:  
> IPA.metadata[entity].object_name_plural.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
ACK.  Pushed to master
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20101202/459baa5c/attachment.htm>