On 04/26/2011 03:12 PM, Endi Sukma Dewata wrote:
On 4/26/2011 12:42 PM, Adam Young wrote:
20. The facet groups needs to be customizable. Currently it's stored
in unordered set in entity's facet_group and the order is hard-
coded in get_facet() and facet_tabs(). It would be difficult
to workaround this limitations in custom facets.

See how facets are stored in entity. The names are stored in an
ordered list (that.facets) and the instances are stored in a
dictionary (that.facets_by_name). Facet groups should use a similar
method to store the order and facet collections.

In get_facet() the default facet should be the first facet in the
first group. In facet_tabs() the tab_section should be created
according to the order they are stored.

Agreed, but we have this issue now, and so is not a regression. We'll
fix in a follow on patch.

This is actually a regression. Because of this the Host's and
Service's Managed By is missing. We also want to be able to replace
the facet group names in ACI because they don't quite make sense
(e.g. Role being a member of Privilege) although that's how it's
implemented internally.

As discussed over IRC, patch 221-7 still includes hard-coded facet group names. It may not be an issue for IPA now, but it will be for other projects trying to use the framework.

21. The IPA.entity_header should be created in entity.init()
instead of
in entity.setup() line 305. The entity.setup() only needs to store
the container in the entity.header. This also eliminates manual
creations in the qunit tests.

Long term, agreed, but init doesn't have the container, so under the
current approach, we can't put it there.

It can be done with a simple change. The following code in the
IPA.entity_header can be moved into a create() method which takes a
container parameter:

that.header = $("<div class='entity-header'/>").
append(title(entity)).
append(buttons()).
append(pkey()).
append(search_bar()).
append(entity_container());
container.append(that.header);

The IPA.entity_header should be created together with the entity. It
doesn't need a container. Then the create() method can be called by
IPA.entity_setup(). This way a custom entity can customize the
entity_header.

It's also better to append the DOM object as soon as it's created
instead of appending a complex object into the container. It helps
troubleshooting.

This is not addressed, but it's not a big issue.

22. When hovering above the facet tabs, the mouse changes into a text
cursor instead of pointer.

Not addressed, but not a big issue.

23. Facet tab can change although the page is dirty.
Open Users tab, click one of the users, this will open the Settings
facet. Edit a field, then click another facet, the Dirty dialog box
will appear. Close the dialog box, the tab doesn't change back to
Settings.

Not addressed, but not a big issue.

Merged in patch 222, since this patch does not work with out it.

24. Typo in entity.js and search.js: back_to-seach

This type was done consitently, so it worked.  But It is easy to fix.l
25. Typo in ipa.css: .entity-continer
Not being used.  Removed

26. Incorrect jQuery selector in search.js: entity-tabs.li
Not being used.  removed.

These last two were replaced by a later fix in patch 221 that sets the selected tab more consistently.
27. There's a jslint warning.

Feel free to push if the remaining issues can be fixed before pushing or considered minor.

Fixed


Pushed to master

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to