Re: [Freeipa-devel] [PATCH] Multicolumn enrollment dialog

2010-12-02 Thread 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@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@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK.  Pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] Multicolumn enrollment dialog

2010-12-02 Thread Adam Young

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@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@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel