Re: Code Review

From: Ken Paulsen <Ken.Paulsen_at_Sun.COM>
Date: Tue, 12 Jun 2007 23:17:05 -0700

Hi Ana,

I'm sure the changes are fine, but it's too hard to tell from diffs. If it visually looks good, then you are probably fine. I made comments below about areas that I noticed where different but wasn't sure why... feel free to check in if all the areas below are as you expected.

Thanks!!

Ken

Ana Caballero wrote:

Please review for Bug#3063-login page visual design cleanup
Below are the diffs and login page with images attached. Please let me know if it's okay to checkin. The changes I made were according to the bug comments below:

Visual design cleanup items for the login page:

1) The bottom portion of the screen should continue with the background color at
the bottom of the login (rather than ending and showing the browser default of
white). This should be accomplish by using the "class = LogBdy" call on the
body of the html.

2) The s-curve background image (login-backimage.jpg) is being obscured by some
sort of code object. The image is fine if you look at it by itself but the
bottom portion of the "S" gets obscured when viewing the login page.

3) The s-curve background image (login-backimage.jpg) currently has a border
that is misaligned with the surrounding images resulting in the lack of a border
on the right-hand side of the image. Also the image appears shifted downward by
1 pixel.
Confirm with visual designer if this image is supposed to have a border. If so,
re-align the existing image so that it is oriented properly. If the image is
not supposed to have a border, acquire new image from visual designer, replace,
and orient properly.

4) Login pushbutton should be left-justified with the user and pw text fields.

Thanks,
Ana

Index: login.jsf
===================================================================
RCS file: /cvs/glassfish/admin-gui/src/docroot/login.jsf,v
retrieving revision 1.9
diff -r1.9 login.jsf
50d49
< <sun:body id="body3" focus="loginform.j_username">
51a51
>     <body id="body3" class="LogBdy" focus="loginform.j_username">
53c53
<     <form method="POST" name="loginform" action="j_security_check" >
---
>     <form method="POST" class="form" name="loginform" action="j_security_check" >
60c60
<             <sun:image icon="DOT" width="1" height="1" />
---
>             <sun:image icon="DOT" width="728" height="1" />

This changes the width to 728. Is that what you want? If so, that's fine... just want to double check.

66,67c66,68
<     <tr class="LogTopBnd" style="background-image: url(#{themeImages.LOGIN_GRADIENT_TOP}); background-repeat: repeat-x; background-position: left top;">
<         <td>
---
>     <tr class="LogTopBnd"><td><sun:image icon="#{themeImages.LOGIN_GRADIENT_TOP}" width="1" height="30" border="0" />

Does this work better as a real image vs. a background image?

>         </td>
>         <td valign="middle" nowrap="nowrap">
72,73c73
<         <td nowrap="nowrap" valign="middle"></td>
<         <td> </td>
---
>         <td></td>
75,76c75,76
<     <tr><td class="LogMidBnd" style="background-image: url(#{themeImages.LOGIN_GRADIENT_SIDES});background-repeat:repeat-x;background-position:left top;"> </td>
<         <td class="LogCntTd" style="background-image: url(#{request.contextPath}/images/login-backimage.jpg);background-repeat:no-repeat;background-position:left top;" height="435" align="center" valign="middle">
---
>             <tr><td class="LogMidBnd"></td>
>         <td class="LogCntTd" style="background-image: url(#{request.contextPath}/images/login-backimage.jpg);background-repeat:no-repeat;background-position:left top;" align="center" valign="middle">

It was intentional to move the image from the first td to the 2nd?

120,121c120,121
<                     <div class="logBtn">
<                         <input type="submit" class="Btn1Def"
---
>                    >                         <input type="submit" class="Btn1"

The class changed from Btn1Def to Btn1.

165a166
> </body>
168c169
< </sun:body>
---
>

"

" "

"

(image/jpeg attachment: 01-part)

(image/jpeg attachment: 02-part)

(image/jpeg attachment: 03-part)