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

[Adam Young]

On 04/22/2011 06:28 PM, Endi Sukma Dewata wrote:
> On 4/22/2011 2:33 PM, Adam Young wrote:
>>>> Again, have not yet run Selenium against this, so please do not push.
>>>> There are conflicts between this version and some of edewata's patch.
>>>> Additionally, there are some know issues with the rendering on the
>>>> ACI pages which I'll iron out before this gets submitted for real.
>>>>
>>>> This version solves Issues 1,2,4,5 (sort of) ,8,9,and 10 from below.
>
>>> This version deals with #7. Unit tests and jsl is fixed. Rebased on
>>> top of Endi's last action-button patch.
>>>
>>> Remaining issues have to do with css and styling.
>>>
>>> Still haven't run it through the Selenium tests.
>
>> OK, with this, the most egregious of the UI issues are fixed. While I'm
>> sure we'll want to do more with it in the long term, I'm going to say
>> that this is ready to go in. We'll fix the selenium tests as a follow on
>> patch.
>
> Some issues:
>
> 3. Default tab is not activated.
>    Open Users, click one of the user, the default tab is activated.
>    Click Back to List, open the user again, the default tab is not
>    activated.
Fixed
>
> 4. Inconsistent position of the action buttons.
>    - Open Users tab, observe the position of the Delete & Add buttons.
>      Then click one of the users, the Reset & Update buttons move to
>      the left.
>    - Open User Groups tab, observe the Add buttons, click one of the
>      groups, the Enroll button moves to the left.
Fixed.  Had to remove some of the style info in ipa.css, but I think it 
was only necessary for action-panels

>
> 5. Entity label should be used instead of entity name as the page title.
>    Open Sudo -> Sudo Command Groups -> group1. The page title is
>    "SUDOCMDGROUP:GROUP1". The problem with this is that unlike the
>    label, entity name will not be translated.
Done.  Long term, we need both a Singular and Plural form 
internationalized.  The name field is the single value.

>
> 9. These facet.entity_header assignments are unnecessary:
>    - entity.js:307
>    - details_tests.js:211
>    - entity_tests.js:79
>    Each facet has a reference to the entity, so the entity header can
>    be accessed using that.entity.header inside the facet or
>    facet.entity.header outside the facet.
Leaving them for now.  I am not sure that we are going to navigate by 
way of the entity to the header for always.  Changing it broke the unit 
tests.  I'll address this in a follow on patch that will simplify the 
facet API.

>
> 11. Navigation error.
>     Open User Groups tab, click one of the groups. Under Member Users,
>     click one of the users. It will show an error dialog box (it has
>     some typos too).
>
Still a problem.

>
> 12. In the search facet there is a big space between the buttons and
>     the results table.
>
Intentional.  That is the space for the tabs.  Easy to remove in the 
future,  but want it in there for now.

> 13. The 3rd level tabs appear only when the HBAC, Sudo, and RBAC tabs
>     are selected. This is causing the rest of the page to shift down.
>     As I mentioned before, here is my suggestion:
>     http://edewata.fedorapeople.org/images/mock1.png
>     http://edewata.fedorapeople.org/images/mock2.png
Going to leave it like this for now.  Don't think what you have in the 
mock ups is quite right:  We'll run past UXD and adjust


>
> 14. According to the spec the buttons should be located below the 4th
>     level tabs (facet tabs). So it should be inside a 'facet-header'
>     instead of 'entity-header'.
Spec needs work.  It is easy enough to move things around if we decided 
we want to, but last conversation had the buttons above the tabs.


>
> 15. Most of navigation links are on the left part of the page, but
>     'Back to Link' is on the right side. It's a bit inconvenient having
>     to go all the way to the right just to go back to the search page,
>     especially when reviewing many entries.

Agree that it is a bit off, but it is the spec.  We can revise in the 
future.

>
> 16. Some facet group headers in the 4th level tabs are unnecessary.
>     The Settings header above the Settings tab is redundant.
>     The Member header above DNS resource record tab is probably
>     inappropriate although that's how it's implemented internally.
>     There should be a way not to show the headers.
They are there for spacing.  Once UXD reviews, we can remove them.

>
> 17. Association facet doesn't set page title.
>     Open User Groups, the page title is "USER GROUPS". Click one of the
>     groups, it will open an association facet, the page title is
>     unchanged. Then click Settings tab, the title changes to
>     "GROUP:...". Click the association again, the title remains the
>     same.

Fixed
>
> 18. Kerberos Ticket Policy and Configuration tabs don't have page
>     titles.
>
Fixed

> 19. There should be a space after colon in the page title.
>     Open Users tab, then click admin. The title is "USER:ADMIN".
>
Fixed

> 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.
>
> 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.

>
> For issues #12-16 I think we need to get UXD's feedback.
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-admiyo-0221-4-action-panel-to-top-tabs.patch
Type: text/x-patch
Size: 67058 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110425/1865ecf1/attachment.bin>