Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
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 = $("").
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
Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
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 = $("").
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
25. Typo in ipa.css: .entity-continer
26. Incorrect jQuery selector in search.js: entity-tabs.li
27. There's a jslint warning.
Feel free to push if the remaining issues can be fixed before pushing or
considered minor.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
There are still some issues. Some of them have been discussed over IRC:
On 4/25/2011 3:24 PM, Adam Young wrote:
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.
OK, got that one fixed in this version as well (version 5)
Still a problem. Open HBAC -> HBAC Rule, click one of the rules, an
error dialog box will appear.
18. Kerberos Ticket Policy and Configuration tabs don't have page
titles.
Fixed
These titles end with a colon.
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.
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 = $("").
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.
22. When hovering above the facet tabs, the mouse changes into a text
cursor instead of pointer.
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.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
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. 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. 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. 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. 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). 12. In the search facet there is a big space between the buttons and the results table. 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 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'. 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. 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. 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. 18. Kerberos Ticket Policy and Configuration tabs don't have page titles. 19. There should be a space after colon in the page title. Open Users tab, then click admin. The title is "USER:ADMIN". 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. 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. For issues #12-16 I think we need to get UXD's feedback. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
Good points all. Only one minoir quibble, in-line. On 04/19/2011 05:31 PM, Endi Sukma Dewata wrote: On 4/19/2011 3:07 PM, Adam Young wrote: going to post this, but with a request to hold on pushing to the repo. I have not yet tested against selenium, and suspect that it will break all selenium test navigation. I've tested this patch with static data. There are some issues: 1. Incorrect data after switching tabs. Open Users tab, then click User Groups tab, all of the groups will be admin's. Refresh the page, it will show the correct data. 2. Missing third level tabs. Open HBAC tab, the Services & Service Groups are missing. Open SUDO tab, the Commands and Command Groups tabs are missing. Open Role Based Access Control, the Permissions and Privileges tabs are missing. 3. Default tab is not activated. Open Users->admin, the Settings tab is inactive. It should be bigger than the other tabs. 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. 5. Entity label (e.g. Users) should be used instead of entity name (e.g. user) as the page title (next to the buttons). 6. The page title (e.g. user) is too close to the tab groups (e.g. Settings). It needs some space between them. 7. The order of tab groups is not very intuitive. Open Groups tab, click one of the groups. The default tab group (i.e. Settings) is located between Member and Member Of. It would be better to put the default tab at the left most position. While in general your observations are spot on, I'd like to point out that this is intentional, but that the goal will be to always open the leftmost tab. This will allow us to make entities that are primarily containers, like netgroups, as well as dns zone, default to the page that shows the most common use case: manage contained entities. 8. The if-then clause on entity.js:46 is unnecessary. There's no need to check spec.facet_group before assigning it to that.facet_group. 9. The assignment on entity.js:296 is unnecessary. Each facet has a reference to the entity, so the entity header can be accessed using that.entity.header. 10. IPA.entity_header() should take a spec object instead of attribute list for consistency. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0221-action-panel-to-top-tabs
On 4/19/2011 3:07 PM, Adam Young wrote: going to post this, but with a request to hold on pushing to the repo. I have not yet tested against selenium, and suspect that it will break all selenium test navigation. I've tested this patch with static data. There are some issues: 1. Incorrect data after switching tabs. Open Users tab, then click User Groups tab, all of the groups will be admin's. Refresh the page, it will show the correct data. 2. Missing third level tabs. Open HBAC tab, the Services & Service Groups are missing. Open SUDO tab, the Commands and Command Groups tabs are missing. Open Role Based Access Control, the Permissions and Privileges tabs are missing. 3. Default tab is not activated. Open Users->admin, the Settings tab is inactive. It should be bigger than the other tabs. 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. 5. Entity label (e.g. Users) should be used instead of entity name (e.g. user) as the page title (next to the buttons). 6. The page title (e.g. user) is too close to the tab groups (e.g. Settings). It needs some space between them. 7. The order of tab groups is not very intuitive. Open Groups tab, click one of the groups. The default tab group (i.e. Settings) is located between Member and Member Of. It would be better to put the default tab at the left most position. 8. The if-then clause on entity.js:46 is unnecessary. There's no need to check spec.facet_group before assigning it to that.facet_group. 9. The assignment on entity.js:296 is unnecessary. Each facet has a reference to the entity, so the entity header can be accessed using that.entity.header. 10. IPA.entity_header() should take a spec object instead of attribute list for consistency. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
