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

2014-06-10 Thread Petr Vobornik

On 10.6.2014 01:52, Endi Sukma Dewata wrote:

On 6/9/2014 8:46 AM, Petr Vobornik wrote:

I've fixed issues #4, #2, #20 and #18. Commits in the branch, no rebase.

With these 4 changes we are ready for the push. I'll squash them, if
necessary.


You mean #11 instead of #2? The fixes are confirmed.


Yes #11, sorry for the confusion. Fixed for issue #20 was squashed into 
patch #640.


Pushed to master:
* ff17af16e7ba953a70c434fea1dbe128810f6028 webui: remove logout.html
* b577b3d3656f51edf0c33bdb00863c03b11ae512 webui: remove login.html
* 6a8eeff22d98c8c32c770869427883198278d077 webui: add PaternFly css
* 78f026bc9013347d4bc2b4c02e72b19495a1b8ac webui: apply PatternFly login 
theme on reset_password.html
* 1829fa2c1571428cb4318443387dde1707fc9641 webui: apply PatternFly theme 
on config pages

* 5a2aed99baa059e9ccdfd9f0d2f2b4cb68ba8930 webui: styles for alert icons
* f0cf2e10d5ca6ce261c256288e2b6d15b23b1418 webui: apply PatternFly theme 
on migration pages
* b5ebdb604bd2c6edbbd99bbd7f21306cbf367412 webui: remove remnants of 
jquery-ui

* 6b5b9a118522a53bca40532aa2df326d0f7bfe38 webui: remove unused icons
* 563dcdc3ebbc11cd979454e75e664767f9ea43f8 webui: remove unused 
collapsible feature from section

* 7e94ee11eb377c8d4ec72e04f618c76f0a30e7a4 webui: remove unused images
* 4333161ac36c9487f225d61c79d8ff904f51d629 webui: change absolutely 
positioned layout to fluid
* 0e15a282e85b0e3eb71fd3fce1965646aeb47a27 webui: remove column sizing 
in tables, use PF styles
* 3eaa69a68681a2478a6feeff7fb9e4cf2a27deee webui: change navigation from 
RCUE to PatternFly
* 2e9e5792bc7c5fd1ed65b4d72e3915442db060f5 webui: adjust styles to 
PatternFly
* bcb2ce7f2464281923dd54396fa18d62e48ffebe webui: display undo and 
multivalued delete buttons in input-group
* 216e710188279d15c262da907efbc09be92fb50a webui: allow multiple base 
section layouts
* ad338b9d74fd3aa22bce5680b39fdaae3b90d5ca webui: change breadcrumb to 
PatternFly
* 3dd34d6e55ae5d671158e2fa8f0213072d897036 webui: use h1 in facet title 
instead of h3

* fc0926ba91f57cb5cd182f2edf50f24d4cfd7432 webui: remove action list widget
* c7af2458091ba95eccc26f4468234413e8b016b9 webui: add action dropdown
* ec9539d0fde6c368e15c0a07ff75f82adba3f36e webui: add space between 
action buttons's icon and text

* be3aadd06ad2cdc434827e78e5227f34ecf63aa0 webui: remove select action
* a98df325b6b0d7bf41509e8c8247a9422f179429 webui: add confirmation to 
action dropdown actions
* 4e1c0ad423b0b78f620b72a07d2c174ec76d4a82 webui: move certificate 
actions to action dropdown
* 2f3dc7908d1c62b729ab38b6d684dc0e942c4528 webui: move user reset 
password action to action dropdown

* faf4fea30fd01ad5f5c372877d0e8fe20963dc91 webui: patternFly dialog
* dff5f6319fd76d6b67b30be4eadbcdb414784802 webui: adjust association 
adder dialog to PatternFly

* f631b07507d12d2ab5c1b535a987f09cb07a5565 webui: activity indicators
* 21651d9d3f463f5c07355d0f4e52a8d3867d88cc webui: improve pagination
* 9c1da611ea94046090dd88d320253d2cded5c76e webui: do not show empty 
table footer
* 54990227824957fb1be4a45ee0f0879812d71d94 webui: restyle automember 
default group
* 4f45e3ea924cd64f4c1f698dfd762d561226199e webui: preload automember 
default group select list
* ea93590ef1051c17fa55115edc14d646581188e4 webui: adjust login page to 
PatternFly
* 74fc85d003af9376462b0610593f6ab8e8a34bf1 webui: use BS alerts in 
validation_summary_widget
* bf9eeb823b5df3e581061b23ee89f3b94b0c87b3 webui-ci: select search table 
item - chrome issue
* 99ed015c0a3ad5f4c1d16190cfa513299ed1cf6c webui: remove old css for 
standalone pages
* 5c3fd4bb832859172fa6e7c739724f6562ccfb7f webui: adjust header controls 
alignment
* 40a25ecf371ebfcbb0801e1f99e3a2853439f44b webui: add search box 
placeholder text
* 408457ce53c553c27f25a682f3f5118ae2e1ab30 webui: change control buttons 
to normal buttons
* 05a917eb17d7c99338dcb972470f15d9d83b37b4 webui: certificate search - 
select search attribute only when defined
* 29f60931e2bee750e5acdf7bead5ed3b21d7e4d5 webui: association adder 
dialog - change find label to filter
* 2df5e0b132be032dc5d12ec65314a44af87b6deb webui: use dark color for 
facet titles without pkey
* 841e0cd3ae904cb8021c4387472b4e66487a5c6a webui-ci: 
assert_action_list_action
* 2af21743df2e09cabcf4d2209cd35786ed3cfdd6 webui: move host action panel 
actions to action dropdown
* 254b41e485e065b870205bb1bc50701451800bff webui: move service action 
panel actions to action dropdown
* dd69557f4e51aa0ab35d82a5c1f67cea20303e70 webui: use normal buttons 
instead of link buttons in multivalued widget
* 0fadb14ec7fe84596058e106a5e8068afed81f51 webui: move radius proxy 
action panel commands to header actions

* bedd128de07f502cdc68d8c7a9f6b8ef48d1727b webui: proper alerts in dialogs
* bc6105b270ad10349d12d144b5f67660e7f734e3 webui: use propert alerts in 
header notification area
* dea2da4455b3a4bd189828e2d22f19ab90bf75bd webui: fix search box overlap 
in mobile mode
* 31df435e4194927649fbdacaa3a62fc28b8f5029 webui: fix layout of QR code 
on wide screens

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

2014-06-09 Thread Petr Vobornik

On 6.6.2014 20:35, Endi Sukma Dewata wrote:

On 6/6/2014 10:43 AM, Petr Vobornik wrote:

On 6.6.2014 15:45, Endi Sukma Dewata wrote:

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.


Thank you for the review.

I've fixed issues:

- #19, #20 in patch  #640.

And some low hanging fruit:
- #16, in patch #637
- #17, in patch #612
- #13, in patch #612

The branch has been rebased to current master.


ACK.


I've fixed issues #4, #2, #20 and #18. Commits in the branch, no rebase.

With these 4 changes we are ready for the push. I'll squash them, if 
necessary.






I am not able to reproduce issues #4 and #11.



4. In the list page (e.g. Users) in mobile mode the Refresh button
may overlap the search box.


Here's what I saw as I was adjusting the page width:
http://edewata.fedorapeople.org/ipa/images/snapshot1.png
http://edewata.fedorapeople.org/ipa/images/snapshot2.png
http://edewata.fedorapeople.org/ipa/images/snapshot3.png

Notice that in snapshot #2 the search box is partially covered by the
Refresh button.


11. In desktop mode the QR code for new OTP token is displayed
outside the dialog box.


Here's what I saw:
http://edewata.fedorapeople.org/ipa/images/snapshot4.png


Fixed, both were Firefox-only issues.




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


caused by BS' CSS:
code {
white-space: nowrap;
}

I wonder if the best solution is to reset it to initial value in all
dialogs.


Alternatively, the sample command could be broken into two lines.


IMHO the style is better, because it's more general and easier for 
copypaste. I've also added word-wrap to break very long subjects.





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


Fixed


The View certificate is still inconsistent though.



Shame on me :)

--
Petr Vobornik

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


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

2014-06-09 Thread Endi Sukma Dewata

On 6/9/2014 8:46 AM, Petr Vobornik wrote:

I've fixed issues #4, #2, #20 and #18. Commits in the branch, no rebase.

With these 4 changes we are ready for the push. I'll squash them, if
necessary.


You mean #11 instead of #2? The fixes are confirmed.


2. If there's a login error, the logo and the message shifts up. I
think it would be nicer to display the error without changing the
layout of the original page.


Snapshots for #2, just in case:
http://edewata.fedorapeople.org/ipa/images/snapshot5.png
http://edewata.fedorapeople.org/ipa/images/snapshot6.png

--
Endi S. Dewata

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


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

2014-06-06 Thread Endi Sukma Dewata

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

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


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

2014-06-06 Thread Petr Vobornik

On 6.6.2014 15:45, Endi Sukma Dewata wrote:

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.


Thank you for the review.

I've fixed issues:

- #19, #20 in patch  #640.

And some low hanging fruit:
- #16, in patch #637
- #17, in patch #612
- #13, in patch #612

The branch has been rebased to current master.

Some comments bellow. For the rest (including issues #1-#12) I'll create 
tickets and/or fix them later. Some are existing issues.


I am not able to reproduce issues #4 and #11.



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.



Fixed. Also disabled items are no longer focusable.


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.


I don't think this page is cached - the wrong command is executed every 
time which will show the dialog.




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


Fixed



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


Fixed DropdownWidget's _render_items method.



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


caused by BS' CSS:
code {
   white-space: nowrap;
}

I wonder if the best solution is to reset it to initial value in all 
dialogs.




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');


Fixed



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


Fixed



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


--
Petr Vobornik

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


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

2014-06-06 Thread Endi Sukma Dewata

On 6/6/2014 10:43 AM, Petr Vobornik wrote:

On 6.6.2014 15:45, Endi Sukma Dewata wrote:

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.


Thank you for the review.

I've fixed issues:

- #19, #20 in patch  #640.

And some low hanging fruit:
- #16, in patch #637
- #17, in patch #612
- #13, in patch #612

The branch has been rebased to current master.


ACK.


I am not able to reproduce issues #4 and #11.



4. In the list page (e.g. Users) in mobile mode the Refresh button may overlap 
the search box.


Here's what I saw as I was adjusting the page width:
http://edewata.fedorapeople.org/ipa/images/snapshot1.png
http://edewata.fedorapeople.org/ipa/images/snapshot2.png
http://edewata.fedorapeople.org/ipa/images/snapshot3.png

Notice that in snapshot #2 the search box is partially covered by the 
Refresh button.



11. In desktop mode the QR code for new OTP token is displayed outside the 
dialog box.


Here's what I saw:
http://edewata.fedorapeople.org/ipa/images/snapshot4.png


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


caused by BS' CSS:
code {
white-space: nowrap;
}

I wonder if the best solution is to reset it to initial value in all
dialogs.


Alternatively, the sample command could be broken into two lines.


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


Fixed


The View certificate is still inconsistent though.

--
Endi S. Dewata

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


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

2014-06-05 Thread Endi Sukma Dewata

ACK for patches #592-#628. I'll continue reviewing the rest.

There are some minor/unrelated issues I found while testing but they can 
be addressed separately:


1. In the login page the message says To login with username and 
password, enter them in the fields below, but the fields are actually 
displayed on the left of the message in desktop mode (big screen) or 
above the message in mobile mode (small screen).


2. If there's a login error, the logo and the message shifts up. I think 
it would be nicer to display the error without changing the layout of 
the original page.


3. In the login page the configured link goes to unauthorized.html 
which displays Unable to verify your Kerberos credentials message. I 
think people expect to see an instruction to configure the browser, not 
an error message.


4. In the list page (e.g. Users) in mobile mode the Refresh button may 
overlap the search box.


5. In mobile mode the list table loses the outer border. Is this 
intentional? If so, maybe the left/right margin could be removed as well 
to maximize the horizontal space.


6. In desktop mode if the page is narrower than a certain width the 
table cell content may wrap (i.e. First name will be displayed in 2 
lines).


7. If the list table is empty, the cell borders disappear (see Host 
Groups page).


8. In mobile mode if you click the menu icon on the top right there will 
be 2 scrollbars, one for the whole page, and one for the menu itself. I 
think in mobile mode the menu should occupy the entire page, so there's 
only one scrollbar.


9. In mobile mode if you select Policy from the menu it will show the 
HBAC Rules page. Similarly, if you click IPA Server it will show the 
Roles page. This can be confusing since in mobile mode we don't see the 
navigation hierarchy. Probably the breadcrumb needs to include the full 
hierarchy (e.g. Policy  HBAC  Rules). Alternatively, the menu items 
that don't have a corresponding page shouldn't be clickable.


10. In mobile mode the Available and Prospective tables in the 
association dialog are arranged vertically, but the Add and Delete 
arrows are still pointing horizontally.


11. In desktop mode the QR code for new OTP token is displayed outside 
the dialog box.


12. There seems to be a caching issue. I added a user, then I added an 
automember rule that will add the user to a group. When I opened the 
user details page initially, the User Groups tab header was showing 
it's a member of 1 group. Then I rebuilt auto membership and reopened 
the page, it's still showing 1 group. Then I clicked another tab (e.g. 
Netgroups) it's now showing 2 groups. But if I clicked User Groups tab 
it went back to 1 group. To get the right info I had to click Refresh. I 
think the counter in the tab header should be based on the cached tab 
content. Alternatively, rebuilding automember should invalidate the cache.


--
Endi S. Dewata

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


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

2014-05-29 Thread Petr Vobornik

On 26.5.2014 17:08, Petr Vobornik wrote:

On 19.5.2014 14:58, Petr Vobornik wrote:

On 12.5.2014 17:46, Misnyovszki Adam wrote:

Hi,
see my review notes below:

On Mon, 05 May 2014 18:41:13 +0200
Petr Vobornik pvobo...@redhat.com wrote:


This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based
PatternFly (v0.2.4) according to plan described at:

http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html

The rest of the patches are mostly response to new CSS styles + some
new functionality and simplification of UI:
- css cleanup, images cleanup
- adjustment of stand-alone pages to PF
- adjustment of DOM structure to Bootstap 3 structure
- BS 3 enabled to change absolute positioned layout to responsive
fluid layout
- new activity indicators (since the old didn't fit into PF
navigation)
- new pager styles + additional behavior
- action select transform into dropdown and moved to control-button
section, making the header responsive
- fluid layout requested removal of computation of columns widths
- removal of login.html and logout.html
- new login background (the old one did not work with PF styles)
- new dialog styles
- + additional adjustments to use PF

The result is that UI uses most of PatternFly styles and is
responsive.

Fixes:
https://fedorahosted.org/freeipa/ticket/4177 - Better indication of
ongoing activity if dialog is opened

  - working progress could have a border. if it is over a dialog,
sometimes it looks messy over text



Fixed


https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on
Cellphone screen

  - when I open the menu in 320x480, and select and navigate to an item,
the menu stays open - needs more investigation, if it is freeipa ui
issue


Fixed


  - qr code is fixed size in otp tokens, doesn't look nice on small
screens
not a problem, user just clicks on qr code link


Fixed


  - when a table header is longer, than the actual screen size, overflow
hidden occurs, unable to use buttons at the end of the header eg DNS
Resource Records, 320x480px, sometimes delete and add button
overflows the table, you can only scroll that table with tap
not a problem, responsive table works this way



I did not encounter overflow hidden issue - scrollbars were present and
I could scroll to the icons.


  - in 320x480, login page configuration text overflows on a white
background, especially if there is a login error, which makes the
white text unreadable


Behavior was improved.




https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display
Loading message when a list of entries is being loaded

see working progress comment above

https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width
limit in UI

ACK - PatternFly 3 handles this very neatly

https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear
which row a value belongs to

ACK - row color alternation hopefully solves the problem

https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme
in config and migration pages

FreeIPA logo doesn't lead anywhere, no way to navigate to the
login page, only by altering the url, or clicking the back button. IMO
logo should always lead to login page if not logged in.


Logo now points to UI


https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and
logout.html

ACK

https://fedorahosted.org/freeipa/ticket/4282


Other issues:
  - unit tests have several fails, possibly because of dom changes


Fixed


  - integration tests ran without errors

Also, according to the UX meeting with Kyle, this patchset should
include the following changes:

  - placeholder for search, box should be on the left
  - actions in one place, on the right in search page
  - actions in one place, on the left in details page
  - action dropdown list to the right near update button in details page
  - left align form fields in details page, two columns arrangement
if the screen is wide
  - hbac details pages - leave it as it is, no form modification
required
  - association adder dialog - placeholder for textbox(Filter
available),
change button text Filter
  - search page title should be changed - use dark variant text
  - multi value list - add to button, with undo all button group
  - multi value list - delete should be also a button
  - left align firefox configuration page steps - ie. every static
page
  - migration should look like login, (~reset_password), text
should go to right
  - error page return back should be a button


All fixed



Thanks
Adam



The suggestions found by UX review resulted in additional 10 new commits
(patch numbers 633-642):

I think, that we should switch from patch files to my git branch to
avoid sending 1-2MB of patches in each review cycle.

http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=patternfly



To be exact:

git log --pretty=format:%s -47
642 webui: use normal buttons instead of link buttons in multivalued
widget
641 webui: move 

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

2014-05-26 Thread Petr Vobornik

On 19.5.2014 14:58, Petr Vobornik wrote:

On 12.5.2014 17:46, Misnyovszki Adam wrote:

Hi,
see my review notes below:

On Mon, 05 May 2014 18:41:13 +0200
Petr Vobornik pvobo...@redhat.com wrote:


This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based
PatternFly (v0.2.4) according to plan described at:

http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html

The rest of the patches are mostly response to new CSS styles + some
new functionality and simplification of UI:
- css cleanup, images cleanup
- adjustment of stand-alone pages to PF
- adjustment of DOM structure to Bootstap 3 structure
- BS 3 enabled to change absolute positioned layout to responsive
fluid layout
- new activity indicators (since the old didn't fit into PF
navigation)
- new pager styles + additional behavior
- action select transform into dropdown and moved to control-button
section, making the header responsive
- fluid layout requested removal of computation of columns widths
- removal of login.html and logout.html
- new login background (the old one did not work with PF styles)
- new dialog styles
- + additional adjustments to use PF

The result is that UI uses most of PatternFly styles and is
responsive.

Fixes:
https://fedorahosted.org/freeipa/ticket/4177 - Better indication of
ongoing activity if dialog is opened

  - working progress could have a border. if it is over a dialog,
sometimes it looks messy over text



Fixed


https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on
Cellphone screen

  - when I open the menu in 320x480, and select and navigate to an item,
the menu stays open - needs more investigation, if it is freeipa ui
issue


Fixed


  - qr code is fixed size in otp tokens, doesn't look nice on small
screens
not a problem, user just clicks on qr code link


Fixed


  - when a table header is longer, than the actual screen size, overflow
hidden occurs, unable to use buttons at the end of the header eg DNS
Resource Records, 320x480px, sometimes delete and add button
overflows the table, you can only scroll that table with tap
not a problem, responsive table works this way



I did not encounter overflow hidden issue - scrollbars were present and
I could scroll to the icons.


  - in 320x480, login page configuration text overflows on a white
background, especially if there is a login error, which makes the
white text unreadable


Behavior was improved.




https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display
Loading message when a list of entries is being loaded

see working progress comment above

https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width
limit in UI

ACK - PatternFly 3 handles this very neatly

https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear
which row a value belongs to

ACK - row color alternation hopefully solves the problem

https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme
in config and migration pages

FreeIPA logo doesn't lead anywhere, no way to navigate to the
login page, only by altering the url, or clicking the back button. IMO
logo should always lead to login page if not logged in.


Logo now points to UI


https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and
logout.html

ACK

https://fedorahosted.org/freeipa/ticket/4282


Other issues:
  - unit tests have several fails, possibly because of dom changes


Fixed


  - integration tests ran without errors

Also, according to the UX meeting with Kyle, this patchset should
include the following changes:

  - placeholder for search, box should be on the left
  - actions in one place, on the right in search page
  - actions in one place, on the left in details page
  - action dropdown list to the right near update button in details page
  - left align form fields in details page, two columns arrangement
if the screen is wide
  - hbac details pages - leave it as it is, no form modification required
  - association adder dialog - placeholder for textbox(Filter available),
change button text Filter
  - search page title should be changed - use dark variant text
  - multi value list - add to button, with undo all button group
  - multi value list - delete should be also a button
  - left align firefox configuration page steps - ie. every static
page
  - migration should look like login, (~reset_password), text
should go to right
  - error page return back should be a button


All fixed



Thanks
Adam



The suggestions found by UX review resulted in additional 10 new commits
(patch numbers 633-642):

I think, that we should switch from patch files to my git branch to
avoid sending 1-2MB of patches in each review cycle.

http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=patternfly


To be exact:

git log --pretty=format:%s -47
642 webui: use normal buttons instead of link buttons in multivalued widget
641 webui: move service action panel actions to action dropdown

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

2014-05-19 Thread Petr Vobornik

On 12.5.2014 17:46, Misnyovszki Adam wrote:

Hi,
see my review notes below:

On Mon, 05 May 2014 18:41:13 +0200
Petr Vobornik pvobo...@redhat.com wrote:


This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based
PatternFly (v0.2.4) according to plan described at:

http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html

The rest of the patches are mostly response to new CSS styles + some
new functionality and simplification of UI:
- css cleanup, images cleanup
- adjustment of stand-alone pages to PF
- adjustment of DOM structure to Bootstap 3 structure
- BS 3 enabled to change absolute positioned layout to responsive
fluid layout
- new activity indicators (since the old didn't fit into PF
navigation)
- new pager styles + additional behavior
- action select transform into dropdown and moved to control-button
section, making the header responsive
- fluid layout requested removal of computation of columns widths
- removal of login.html and logout.html
- new login background (the old one did not work with PF styles)
- new dialog styles
- + additional adjustments to use PF

The result is that UI uses most of PatternFly styles and is
responsive.

Fixes:
https://fedorahosted.org/freeipa/ticket/4177 - Better indication of
ongoing activity if dialog is opened

  - working progress could have a border. if it is over a dialog,
sometimes it looks messy over text



Fixed


https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on
Cellphone screen

  - when I open the menu in 320x480, and select and navigate to an item,
the menu stays open - needs more investigation, if it is freeipa ui
issue


Fixed


  - qr code is fixed size in otp tokens, doesn't look nice on small
screens
not a problem, user just clicks on qr code link


Fixed


  - when a table header is longer, than the actual screen size, overflow
hidden occurs, unable to use buttons at the end of the header eg DNS
Resource Records, 320x480px, sometimes delete and add button
overflows the table, you can only scroll that table with tap
not a problem, responsive table works this way



I did not encounter overflow hidden issue - scrollbars were present and 
I could scroll to the icons.



  - in 320x480, login page configuration text overflows on a white
background, especially if there is a login error, which makes the
white text unreadable


Behavior was improved.




https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display
Loading message when a list of entries is being loaded

see working progress comment above

https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width
limit in UI

ACK - PatternFly 3 handles this very neatly

https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear
which row a value belongs to

ACK - row color alternation hopefully solves the problem

https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme
in config and migration pages

FreeIPA logo doesn't lead anywhere, no way to navigate to the
login page, only by altering the url, or clicking the back button. IMO
logo should always lead to login page if not logged in.


Logo now points to UI


https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and
logout.html

ACK

https://fedorahosted.org/freeipa/ticket/4282


Other issues:
  - unit tests have several fails, possibly because of dom changes


Fixed


  - integration tests ran without errors

Also, according to the UX meeting with Kyle, this patchset should
include the following changes:

  - placeholder for search, box should be on the left
  - actions in one place, on the right in search page
  - actions in one place, on the left in details page
  - action dropdown list to the right near update button in details page
  - left align form fields in details page, two columns arrangement
if the screen is wide
  - hbac details pages - leave it as it is, no form modification required
  - association adder dialog - placeholder for textbox(Filter available),
change button text Filter
  - search page title should be changed - use dark variant text
  - multi value list - add to button, with undo all button group
  - multi value list - delete should be also a button
  - left align firefox configuration page steps - ie. every static
page
  - migration should look like login, (~reset_password), text
should go to right
  - error page return back should be a button


All fixed



Thanks
Adam



The suggestions found by UX review resulted in additional 10 new commits 
(patch numbers 633-642):


I think, that we should switch from patch files to my git branch to 
avoid sending 1-2MB of patches in each review cycle.


http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=patternfly

To be exact:

git log --pretty=format:%s -47
642 webui: use normal buttons instead of link buttons in multivalued widget
641 webui: move service action panel actions to action dropdown
640 webui: move host action panel 

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

2014-05-12 Thread Misnyovszki Adam
Hi,
see my review notes below:

On Mon, 05 May 2014 18:41:13 +0200
Petr Vobornik pvobo...@redhat.com wrote:

 This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based 
 PatternFly (v0.2.4) according to plan described at:
 
 http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html
 
 The rest of the patches are mostly response to new CSS styles + some
 new functionality and simplification of UI:
 - css cleanup, images cleanup
 - adjustment of stand-alone pages to PF
 - adjustment of DOM structure to Bootstap 3 structure
 - BS 3 enabled to change absolute positioned layout to responsive
 fluid layout
 - new activity indicators (since the old didn't fit into PF
 navigation)
 - new pager styles + additional behavior
 - action select transform into dropdown and moved to control-button 
 section, making the header responsive
 - fluid layout requested removal of computation of columns widths
 - removal of login.html and logout.html
 - new login background (the old one did not work with PF styles)
 - new dialog styles
 - + additional adjustments to use PF
 
 The result is that UI uses most of PatternFly styles and is
 responsive.
 
 Fixes:
 https://fedorahosted.org/freeipa/ticket/4177 - Better indication of
 ongoing activity if dialog is opened
 - working progress could have a border. if it is over a dialog,
   sometimes it looks messy over text

 https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on
 Cellphone screen
 - when I open the menu in 320x480, and select and navigate to an item,
   the menu stays open - needs more investigation, if it is freeipa ui
   issue
 - qr code is fixed size in otp tokens, doesn't look nice on small
   screens
   not a problem, user just clicks on qr code link
 - when a table header is longer, than the actual screen size, overflow
   hidden occurs, unable to use buttons at the end of the header eg DNS
   Resource Records, 320x480px, sometimes delete and add button
   overflows the table, you can only scroll that table with tap
   not a problem, responsive table works this way
 - in 320x480, login page configuration text overflows on a white
   background, especially if there is a login error, which makes the
   white text unreadable

 https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display
 Loading message when a list of entries is being loaded
see working progress comment above
 https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width
 limit in UI   
ACK - PatternFly 3 handles this very neatly
 https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear
 which row a value belongs to
ACK - row color alternation hopefully solves the problem
 https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme
 in config and migration pages
FreeIPA logo doesn't lead anywhere, no way to navigate to the
login page, only by altering the url, or clicking the back button. IMO
logo should always lead to login page if not logged in.
 https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and
 logout.html
ACK
 https://fedorahosted.org/freeipa/ticket/4282

Other issues:
 - unit tests have several fails, possibly because of dom changes
 - integration tests ran without errors

Also, according to the UX meeting with Kyle, this patchset should
include the following changes:

 - placeholder for search, box should be on the left
 - actions in one place, on the right in search page
 - actions in one place, on the left in details page
 - action dropdown list to the right near update button in details page
 - left align form fields in details page, two columns arrangement
   if the screen is wide
 - hbac details pages - leave it as it is, no form modification required
 - association adder dialog - placeholder for textbox(Filter available),
   change button text Filter
 - search page title should be changed - use dark variant text
 - multi value list - add to button, with undo all button group
 - multi value list - delete should be also a button
 - left align firefox configuration page steps - ie. every static
   page
 - migration should look like login, (~reset_password), text
   should go to right
 - error page return back should be a button

Thanks
Adam

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