[Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs

[Adam Young]

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