[Freeipa-devel] [PATCH] 592-628 Update to PatternFly

Endi Sukma Dewata edewata at redhat.com
Fri Jun 6 13:45:33 UTC 2014


On 6/5/2014 9:25 AM, Endi Sukma Dewata wrote:
> ACK for patches #592-#628. I'll continue reviewing the rest.

ACK for patches #633-639, #642, #644, #652, and #653. Patches #640 & 
#641 have an issue (see #19 below) that should be fixed before pushing. 
Other issues are minor/unrelated/suggestions that can be addressed 
separately.

13. The separators in the top right drop down menu shouldn't be 
focusable. To test this, click the menu, then hit tab several times.

14. In the certificates page, if I enter a search filter then hit 
Refresh (instead of Enter) it doesn't change the content based on the 
new search filter. I suppose a more intuitive behavior would be to rerun 
the search with the new filter, or reset the filter to the previous value.

15. In the certificates page, if I enter an invalid filter it will show 
an error dialog, then after I close it it will show the error message in 
the page. This is fine, but if I go to another page, then back, it will 
do the same thing as if the search is executed again. If the page is 
cached, it probably shouldn't display the error dialog, just the error 
message in the page. Alternatively, if the search failed it shouldn't be 
cached.

16. In the association adder dialog it's not clear if the filter applies 
to just the Available list or the Prospective list too. Maybe the 
placeholder text could say "Filter available ${other_entity}".

17. It looks like some facet-actions elements contain unnecessary blank 
ul.dropdown-menu elements which probably correspond to the number of 
menu items (see User's Actions button).

18. In the New Certificate dialog for Host, the instruction to create a 
CSR exceeds the dialog boundary.

19. The "View certificate" is missing from the Host's and Service's 
Action, probably because of the incorrect labels below:

   IPA.cert.view_action = function(spec) {

     // should be objects.cert.view_certificate_btn
     spec.label = spec.label || '@i18n:objects.cert.view';

     that.execute_action = function(facet) {

       // should be objects.cert.view_certificate
       var title = text.get('@i18n:objects.cert.view_certificate_btn');

20. The capitalization of "Certificate" is inconsistent in Host's and 
Service's Actions.

21. Should there be a link from the Host page to the Certificate page? 
Can the certificate serial number be displayed in the Host page? If we 
do this, it's probably not necessary to have Revoke/Restore Certificate 
actions in the Host page. Same thing with the Service page.

22. The add dialog for RADIUS Server uses a field label "Secret". 
Everywhere else in the RADIUS Server page it's called "Password" (e.g. 
Verify Password, Reset Password).

-- 
Endi S. Dewata




More information about the Freeipa-devel mailing list