> The banner pic is controlled by css and not directly edited in banner.jsp, if > you have a look at the old banner.jsp, you can see it starts with a "<tr>", > that really tight-coupled with the page it is included. I don't like it.
The pic is controlled by css, that's right. But I noticed there are some hard coded items like 'height' and 'border' and an 'logout' image. I agree with you that starting with the tag '<tr>' is not so good, but how about moving the '<table>' to banner.jsp as well? > Yes, that's common, but not always necessary. We have very limited lines in > banner.jsp and its parent portlets-with-tree.jsp is not that big, take it > easy.. Yes, it's not always necessary. But I still think it's a good practice to follow... 2011/7/20 Rex Wang <[email protected]>: > > > 2011/7/20 Shenghao Fang <[email protected]> >> >> But I thought if we need to modify some texts or pictures on banner, >> keep a separate banner.jsp makes it easier to locate and modify. > > The banner pic is controlled by css and not directly edited in banner.jsp, > if you have a look at the old banner.jsp, you can see it starts with a > "<tr>", that really tight-coupled with the page it is included. I don't like > it. >> >> Although banner.jsp is only included in one jsp, keeping these small >> pieces such as banner, footer, navigation etc. in separated jsps >> instead of included in a big jsp is very common way in web design. > > Yes, that's common, but not always necessary. We have very limited lines in > banner.jsp and its parent portlets-with-tree.jsp is not that big, take it > easy.. > > -Rex > >> >> 2011/7/20 Rex Wang <[email protected]>: >> > I don't want the default-theme.jsp make up of too many small pieces, it >> > doesn't make sense to me, especially the banner.jsp is only included by >> > only >> > one jsp, isn't it? If no other pages include it, we should just delete >> > the >> > banner.jsp >> > >> > thanks, >> > -Rex >> > >> > 2011/7/20 Shenghao Fang <[email protected]> >> >> >> >> It looks to me that 'banner.jsp' was included in >> >> 'portlets-with-tree.jsp' by '<%@ include file="./banner.jsp" %>' >> >> before the change but now the contents are hard coding in >> >> 'portlet-with-tree.jsp'. >> >> >> >> I thought using '<%@ include file="./banner.jsp" %>' make it more >> >> clear and graceful. >> >> >> >> Thanks. >> >> >> >> 2011/7/19 <[email protected]>: >> >> > Author: rwonly >> >> > Date: Tue Jul 19 02:22:28 2011 >> >> > New Revision: 1148127 >> >> > >> >> > URL: http://svn.apache.org/viewvc?rev=1148127&view=rev >> >> > Log: >> >> > GERONIMO-6081 Some admin console UI issues >> >> > >> >> > Modified: >> >> > >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/navigation.jsp >> >> > >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlet-skin.jsp >> >> > >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlets-with-tree.jsp >> >> > >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/main.css >> >> > >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/pluto.css >> >> > >> >> > Modified: >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/navigation.jsp >> >> > URL: >> >> > >> >> > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/navigation.jsp?rev=1148127&r1=1148126&r2=1148127&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/navigation.jsp >> >> > (original) >> >> > +++ >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/navigation.jsp >> >> > Tue Jul 19 02:22:28 2011 >> >> > @@ -54,29 +54,30 @@ limitations under the License. >> >> > >> >> > %> >> >> > >> >> > -<table class="claro" width="200px" border="0" cellpadding="0" >> >> > cellspacing="0"> >> >> > +<table class="claro" width="260px" border="0" cellpadding="0" >> >> > cellspacing="0"> >> >> > <tr> >> >> > - <td CLASS="ReallyDarkBackground"><strong> <fmt:message >> >> > key="Console Navigation"/></strong></td> >> >> > + <td class="ReallyDarkBackground"><strong> <fmt:message >> >> > key="Console Navigation"/></strong></td> >> >> > </tr> >> >> > <tr> >> >> > <td> >> >> > - <div id="modeSwitcher" >> >> > class="<%=isBasicTreeHasValidItem?"modeSwitcher":"hidden"%>"> >> >> > - <input type="radio" name="mode" id ="mode" >> >> > checked="checked" onclick="changeMode()"/><fmt:bundle >> >> > basename="portaldriver"><fmt:message >> >> > key="console.mode.basic"/></fmt:bundle> >> >> > - <input type="radio" name="mode" id ="mode" >> >> > onclick="changeMode()"/><fmt:bundle >> >> > basename="portaldriver"><fmt:message >> >> > key="console.mode.advanced"/></fmt:bundle> >> >> > - </div> >> >> > - </td> >> >> > - </tr> >> >> > - <tr><td> </td></tr> >> >> > - <tr id="tquickLauncher" style="display:none;"> >> >> > - <td> <input id="quickLauncher"></td> >> >> > - </tr> >> >> > - <tr> >> >> > - <td> >> >> > - <div id="navigationTreeBasic"> >> >> > - >> >> > - >> >> > + <!-- mode div --> >> >> > + <div id="modeSwitcher" >> >> > class="<%=isBasicTreeHasValidItem?"padding4":"hidden"%>"> >> >> > + <input type="radio" name="mode" id ="mode" >> >> > checked="checked" onclick="changeMode()"/><fmt:bundle >> >> > basename="portaldriver"><fmt:message >> >> > key="console.mode.basic"/></fmt:bundle> >> >> > + >> >> > + <input type="radio" name="mode" id ="mode" >> >> > onclick="changeMode()"/><fmt:bundle >> >> > basename="portaldriver"><fmt:message >> >> > key="console.mode.advanced"/></fmt:bundle> >> >> > + </div> >> >> > + >> >> > + >> >> > + <!-- quick launcher div --> >> >> > + <div id="tquickLauncher" class="padding4" >> >> > style="display:none;"> >> >> > + <input id="quickLauncher"> >> >> > </div> >> >> > + >> >> > + >> >> > + <!-- tree div --> >> >> > + <div id="navigationTreeBasic"></div> >> >> > <div id="navigationTreeAdvanced"></div> >> >> > + >> >> > </td> >> >> > </tr> >> >> > </table> >> >> > >> >> > Modified: >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlet-skin.jsp >> >> > URL: >> >> > >> >> > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlet-skin.jsp?rev=1148127&r1=1148126&r2=1148127&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlet-skin.jsp >> >> > (original) >> >> > +++ >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlet-skin.jsp >> >> > Tue Jul 19 02:22:28 2011 >> >> > @@ -33,14 +33,11 @@ limitations under the License. >> >> > <pluto:modeAnchor portletMode="edit"/> >> >> > <pluto:modeAnchor portletMode="help"/> >> >> > <!-- Window State Controls --> >> >> > - <pluto:windowStateAnchor windowState="minimized" icon='<%= >> >> > (request.getContextPath() + "/images/controls/min.png")%>' /> >> >> > - <pluto:windowStateAnchor windowState="maximized" icon='<%= >> >> > request.getContextPath() + "/images/controls/max.png"%>'/> >> >> > - <pluto:windowStateAnchor windowState="normal" icon='<%= >> >> > request.getContextPath() + "/images/controls/norm.png"%>'/> >> >> > - <a href="<pluto:url windowState="minimized"/>"><span >> >> > class="min"></span></a> >> >> > - <a href="<pluto:url windowState="maximized"/>"><span >> >> > class="max"></span></a> >> >> > - <a href="<pluto:url windowState="normal"/>"><span >> >> > class="norm"></span></a> >> >> > + <a href="<pluto:url windowState="minimized"/>"><span >> >> > class="minimized"></span></a> >> >> > + <a href="<pluto:url windowState="maximized"/>"><span >> >> > class="maximized"></span></a> >> >> > + <a href="<pluto:url windowState="normal"/>"><span >> >> > class="normal"></span></a> >> >> > <!-- Portlet Title --> >> >> > - <h2 class="title"><fmt:message >> >> > key="<%=(String)request.getAttribute( >> >> > org.apache.pluto.driver.AttributeKeys.PORTLET_TITLE )%>"/></h2> >> >> > + <h2><fmt:message key="<%=(String)request.getAttribute( >> >> > org.apache.pluto.driver.AttributeKeys.PORTLET_TITLE )%>"/></h2> >> >> > </div> >> >> > <div class="body"> >> >> > <pluto:render/> >> >> > >> >> > Modified: >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlets-with-tree.jsp >> >> > URL: >> >> > >> >> > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlets-with-tree.jsp?rev=1148127&r1=1148126&r2=1148127&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlets-with-tree.jsp >> >> > (original) >> >> > +++ >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/WEB-INF/themes/portlets-with-tree.jsp >> >> > Tue Jul 19 02:22:28 2011 >> >> > @@ -140,57 +140,49 @@ setInterval('autoCheckIframe()',500); >> >> > </ul> >> >> > </div> >> >> > <!-- end accessibility prolog --> >> >> > -<table width="100%" cellpadding="0" cellspacing="0" border="0" >> >> > id="rootfragment"> >> >> > - >> >> > - <!-- Header --> >> >> > - <%@ include file="./banner.jsp" %> >> >> > >> >> > +<!-- Header --> >> >> > +<table width="100%" height="86" border="0" cellpadding="0" >> >> > cellspacing="0"> >> >> > <tr> >> >> > - <td> >> >> > - <table width="100%" border="0" cellpadding="0" >> >> > cellspacing="0"> >> >> > - <!-- Spacer --> >> >> > - <tr> >> >> > - <td class="Gutter"> </td> >> >> > - <td> </td> >> >> > - <td class="Gutter"> </td> >> >> > - <td> </td> >> >> > - <td class="Gutter"> </td> >> >> > - </tr> >> >> > - >> >> > - <!-- Start of Body --> >> >> > - <tr> >> >> > - <!-- Navigation Column --> >> >> > - <td class="Gutter"> </td> <!-- Spacer --> >> >> > - <td width="200px" class="Selection" >> >> > valign="top"> >> >> > - <div id="left-nav"> >> >> > - <table width="100%" border="0" >> >> > cellpadding="0" cellspacing="0"> >> >> > - <tr> >> >> > - <td > >> >> > - <!-- Include Navigation.jsp >> >> > here --> >> >> > - <jsp:include >> >> > page="navigation.jsp"/> >> >> > - </td> >> >> > - </tr> >> >> > - </table> >> >> > - </div> >> >> > - </td> >> >> > + <td height="86" class="Logo" border="0"></td> >> >> > + <td height="86" class="Top" border="0"> </td> >> >> > + <td height="86" class="Top" border="0" width="40"></td> >> >> > + <td height="86" class="Top" border="0" width="40"> >> >> > + <a href="<%=request.getContextPath()%>/logout.jsp"><img >> >> > border="0" >> >> > src="<%=request.getContextPath()%>/images/head_logout_63x86.gif" >> >> > alt="Logout"/></a> >> >> > + </td> >> >> > + </tr> >> >> > +</table> >> >> > >> >> > - <!-- Portlet Section --> >> >> > - <td class="Gutter"> </td> <!-- Spacer --> >> >> > - <td valign="top"> >> >> > - >> >> > - <iframe src="" id="portletsFrame" width="100%" >> >> > height="100%" scrolling="no" frameborder="0"> >> >> > - >> >> > - </iframe> >> >> > - </td> >> >> > +<p style="margin-top:5px;margin-bottom:5px" /> >> >> > >> >> > - <td class="Gutter"> </td> <!-- Spacer --> >> >> > - <td class="Gutter"> </td> <!-- Spacer --> >> >> > - </tr> >> >> > - <!-- End of Body --> >> >> > - </table> >> >> > +<!-- Body --> >> >> > +<table width="100%" border="0" cellpadding="0" cellspacing="0"> >> >> > + <tr> >> >> > + <!-- Spacer --> >> >> > + <td class="Gutter"> </td> >> >> > + >> >> > + <!-- Navigation Column --> >> >> > + <td width="260px" class="Selection" valign="top"> >> >> > + <div id="left-nav"> >> >> > + <!-- Include Navigation.jsp here --> >> >> > + <jsp:include page="navigation.jsp"/> >> >> > + </div> >> >> > + </td> >> >> > + >> >> > + <!-- Spacer --> >> >> > + <td class="Gutter"> </td> >> >> > + >> >> > + <!-- Portlet Section --> >> >> > + <td valign="top"> >> >> > + <iframe src="" id="portletsFrame" width="100%" >> >> > height="100%" scrolling="no" frameborder="0"> >> >> > + </iframe> >> >> > </td> >> >> > + >> >> > + <!-- Spacer --> >> >> > + <td class="Gutter"> </td> >> >> > </tr> >> >> > </table> >> >> > + >> >> > </body> >> >> > <script type="text/javascript"> >> >> > <% >> >> > >> >> > Modified: >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/main.css >> >> > URL: >> >> > >> >> > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/main.css?rev=1148127&r1=1148126&r2=1148127&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/main.css >> >> > (original) >> >> > +++ >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/main.css >> >> > Tue Jul 19 02:22:28 2011 >> >> > @@ -149,23 +149,16 @@ a:hover >> >> > font-size: 1.5em !important; >> >> > } >> >> > >> >> > -.modeSwitcher { >> >> > - margin-bottom: 2px; >> >> > - margin-left: 12px; >> >> > - margin-right: 5px; >> >> > - margin-top: 6px; >> >> > - padding-bottom: 2px; >> >> > - padding-left: 2px; >> >> > - padding-right: 2px; >> >> > - padding-top: 2px; >> >> > +.padding4 { >> >> > + padding: 4px; >> >> > } >> >> > >> >> > #navigationTreeAdvanced { >> >> > - padding-left: 4px; >> >> > + padding: 4px; >> >> > } >> >> > >> >> > #navigationTreeBasic { >> >> > - padding-left: 4px; >> >> > + padding: 4px; >> >> > } >> >> > >> >> > >> >> > @@ -185,6 +178,7 @@ a:hover >> >> > color: #FFFFFF; >> >> > height: 18px; >> >> > line-height: 18px; >> >> > + font-weight: bold; >> >> > } >> >> > >> >> > .Content .Title a:link, >> >> > >> >> > Modified: >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/pluto.css >> >> > URL: >> >> > >> >> > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/pluto.css?rev=1148127&r1=1148126&r2=1148127&view=diff >> >> > >> >> > >> >> > ============================================================================== >> >> > --- >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/pluto.css >> >> > (original) >> >> > +++ >> >> > >> >> > geronimo/server/trunk/plugins/console/console-portal-driver/src/main/webapp/pluto.css >> >> > Tue Jul 19 02:22:28 2011 >> >> > @@ -158,7 +158,7 @@ a.tooltip:hover span.tooltip { >> >> > .portlet { >> >> > margin: 0px 0px 10px 0px; >> >> > padding: 0px; >> >> > - border: 1.5px solid #CCCCCC; >> >> > + >> >> > } >> >> > >> >> > .portlet .header { >> >> > @@ -167,21 +167,17 @@ a.tooltip:hover span.tooltip { >> >> > background-color: #000000; >> >> > color: #FFFFFF; >> >> > font-weight: bold; >> >> > - border-width: 0px 0px 1px 0px; >> >> > - border-style: solid; >> >> > - border-color: #2E6794; >> >> > } >> >> > >> >> > .portlet .header h2 { >> >> > font-family: Verdana, Tahoma, Arial, Helvetica, sans-serif; >> >> > - /*font-size: 12px;*/ >> >> > height: 18px; >> >> > line-height: 18px; >> >> > margin: 0px; >> >> > padding: 0px; >> >> > - /*font-family: "Trebuchet MS", Trebuchet, Arial, Helvetica, >> >> > Sans-serif;*/ >> >> > - font-size: 100%; >> >> > + font-size: 14px; >> >> > float: none; >> >> > + font-weight: bold; >> >> > } >> >> > >> >> > .portlet .header span { >> >> > @@ -219,6 +215,9 @@ a.tooltip:hover span.tooltip { >> >> > >> >> > .portlet .body { >> >> > padding: 10px; >> >> > + border-width: 0px 1px 1px 1px; >> >> > + border-style: solid; >> >> > + border-color: #cccccc; >> >> > } >> >> > >> >> > >> >> > >> >> > >> >> > >> >> >> >> >> >> >> >> -- >> >> Michael >> > >> > >> > >> > -- >> > Lei Wang (Rex) >> > rwonly AT apache.org >> > >> >> >> >> -- >> Michael > > > > -- > Lei Wang (Rex) > rwonly AT apache.org > -- Michael
