Thanks for sensible comments.. Responses inlined...
Jesus M. Rodriguez wrote:
comments inline
On Mon, May 4, 2009 at 7:22 PM, Partha Aji <[email protected]> wrote:
New commits:
commit b536222d2435dd1b4ad7a69ce5f6a800777a6e87
Author: Partha Aji <[email protected]>
Date: Mon May 4 19:20:58 2009 -0400
444221 - More fixes related to snippet pages in general
diff --git a/branding/css/blue-docs.css b/branding/css/blue-docs.css
index 3589d7d..20df51a 100644
--- a/branding/css/blue-docs.css
+++ b/branding/css/blue-docs.css
@@ -226,12 +226,17 @@ div.important
h2{background-image:url(../images/important.png)}
.section .table table tr.even td
{background-color:#f5f5f5;}
-.chapter .table table th p:first-child,table td p:first-child,table li
p:first-child,
-.book .table table th p:first-child,table td p:first-child,table li
p:first-child,
-.preface .table table th p:first-child,table td p:first-child,table li
p:first-child,
-.section .table table th p:first-child,table td p:first-child,table li
p:first-child
+.chapter .table table th p:first-child, .chapter table td p:first-child,
+.chapter table li p:first-child,
+.book .table table th p:first-child, .book table td p:first-child,
+.book table li p:first-child,
+.preface .table table th p:first-child, .preface table td p:first-child,
+.preface table li p:first-child,
+.section .table table th p:first-child, .section table td p:first-child,
+.section table li p:first-child
{margin-top:0em;padding-top:0em;display:inline;}
make sure satellite branding gets similar fixes.
Will do..
+
.chapter th, .chapter td,
.book th, .book td,
.preface th, .preface td,
diff --git a/branding/css/rhn-basic.css b/branding/css/rhn-basic.css
index 9cdbebd..ac61943 100644
--- a/branding/css/rhn-basic.css
+++ b/branding/css/rhn-basic.css
@@ -518,6 +518,13 @@ ul.widespace li{
list-style: none;
}
+.file-display {
+ overflow: auto;
+ max-width: 600px;
+ max-height: 200px;
+ padding: 7px 5px;
+}
+
div.page-summary {
margin: 0 2%;
cursor: text;
diff --git a/java/code/src/com/redhat/rhn/common/util/DynamicComparator.java
b/java/code/src/com/redhat/rhn/common/util/DynamicComparator.java
index 5483ce8..2d55257 100644
--- a/java/code/src/com/redhat/rhn/common/util/DynamicComparator.java
+++ b/java/code/src/com/redhat/rhn/common/util/DynamicComparator.java
@@ -41,8 +41,8 @@ public class DynamicComparator implements Comparator {
private int order;
private Collator collator;
/**
- * Create a new DynamicComparitor that
- *
+ * Create a new DynamicComparator that
+ * can be used to compare indivdual beans..
* @param fieldNameIn Name of field you want to use in
* the bean to compare to
*
@@ -50,14 +50,26 @@ public class DynamicComparator implements Comparator {
* <code>RequestContext.LIST_SORT_DESC</code>
*/
public DynamicComparator(String fieldNameIn, String sortOrder) {
+ this (fieldNameIn, RequestContext.SORT_ASC.equals(sortOrder));
+ }
+
+ /**
+ * Create a new DynamicComparator that
+ * can be used to compare indivdual beans..
+ * @param fieldNameIn Name of field you want to use in
+ * the bean to compare to
+ *
+ * @param ascending true for ascending order
+ */
+ public DynamicComparator(String fieldNameIn, boolean ascending) {
this.fieldName = fieldNameIn;
- if (RequestContext.SORT_ASC.equals(sortOrder)) {
+ if (ascending) {
order = 1;
}
else {
order = -1;
}
- }
+ }
Can we get a unit test for the comparator? Sure it looks like a trivial class,
but let's test it so that we don't break it.
This is not a new add, we have been using DynamicComparator class in the
List Tag over the past few years. All I did was add another
constructor... Sure can add a unit test..
/**
* {...@inheritdoc}
diff --git
a/java/code/src/com/redhat/rhn/domain/kickstart/cobbler/CobblerSnippet.java
b/java/code/src/com/redhat/rhn/domain/kickstart/cobbler/CobblerSnippet.java
index 60db86e..1cc97db 100644
--- a/java/code/src/com/redhat/rhn/domain/kickstart/cobbler/CobblerSnippet.java
+++ b/java/code/src/com/redhat/rhn/domain/kickstart/cobbler/CobblerSnippet.java
@@ -25,8 +25,6 @@ import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.builder.HashCodeBuilder;
import java.io.File;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
/**
* CobblerSnippet - Class representation of a Cobbler snippet
@@ -35,8 +33,7 @@ import java.util.regex.Pattern;
public class CobblerSnippet implements Comparable<CobblerSnippet> {
private File path;
private Org org;
-
-
+
/**
* Method to return the main cobbler snippets dir
* (i.e. /var/lib/cobbler/snippets) .. Reason this is
@@ -289,10 +286,8 @@ public class CobblerSnippet implements
Comparable<CobblerSnippet> {
}
private static void validateFileName(String name) {
- // only [a-zA-Z_0-9] and '.' and '-' are valid filename characters
- Pattern p = Pattern.compile("^[\\w\\.\\-_]+$");
- Matcher m = p.matcher(name);
- if (StringUtils.isBlank(name) || !m.matches()) {
+ // file names can have no slashes can be anything else though..
change the comment to be // filenames can not have slashes.
Because it is incorrect to state it can be anything else, as you already
rule out blank and starts with '.'.
good idea will do..
+ if (StringUtils.isBlank(name) || name.contains("/") ||
name.startsWith(".")) {
ValidatorException.raiseException("cobbler.snippet.invalidfilename.message");
}
}
diff --git
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDeleteAction.java
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDeleteAction.java
index fd90ccf..ed96b71 100644
---
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDeleteAction.java
+++
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDeleteAction.java
@@ -39,7 +39,7 @@ public class CobblerSnippetDeleteAction extends RhnAction {
HttpServletRequest request,
HttpServletResponse response) {
RequestContext context = new RequestContext(request);
- CobblerSnippet snip = CobblerSnippetDetailsAction.getSnippet(request);
+ CobblerSnippet snip = CobblerSnippetDetailsAction.getSnippet(request,
false);
why are we asking an Action for a snippet? I've never seen us make dependencies
between actions. Seems like this should be in a Manager, a Helper class or just
simply operate on the RequestContext directly i.e. (CobblerSnippet)
ctx.getParam("cobbler.snippet")
something along those lines. I'm not a big fan of the DeleteAction
calling back on the
DetailsAction. I'm also going to assume that these Actions are NOT
maintaining state because
that would be a no-no.
Here is the issue. Name is changeable even on the edit page for a
CobblerSnippet. So this changes the logic a little bit..
For our logic to work we need to preseve the OldName as a formVar and
then do things like if !oldName.equals(name) change the name of the
snippet and if there is an error complain and load the original snippet
using the oldName and not the new one (for that doesn;t exist as yet..)
Also this is in Action becasue its using Reuqest and RequestCOntext.. I
think of manager layer that deals with logic that is entirely based on
non UI objects so that one can use it in XMLRPC layer also.. To me
RequestContexts being used in Manager is a code smell. In this case
getSnippet() method uses request heavily. Initially it was a private
method in this class but then I had to copy paste the same exact method
over to the Delete Confirm page (since I had to load a snippet and show
its contents) and at that point was uing the same request parameters. So
I said lets have the loading logic on one action. To mehaving a static
method in a Action is acceptable as long as it deals with UI objects +
and does no state member variable magic (as in only deals with objects
passed to it).. One argument that could ve been made was that this
should be a default access method and not a public static, I am going to
change to default/package acceess ...
request.setAttribute(CobblerSnippetDetailsAction.NAME, snip.getName());
request.setAttribute(CobblerSnippetDetailsAction.PREFIX,
snip.getPrefix());
@@ -48,8 +48,7 @@ public class CobblerSnippetDeleteAction extends RhnAction {
snip.delete();
createSuccessMessage(request,
"cobblersnippet.delete.success", snip.getName());
- return
getStrutsDelegate().forwardParam(mapping.findForward("success"),
- CobblerSnippetDetailsAction.NAME, snip.getName());
+ return mapping.findForward("success");
are there any formvars you need to pass along or keep?
No form vars, they are calling a general list page.. That change was
intentional.
}
catch (ValidatorException ve) {
getStrutsDelegate().saveMessages(request, ve.getResult());
diff --git
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDetailsAction.java
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDetailsAction.java
index 684e193..cd27b6a 100644
---
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDetailsAction.java
+++
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetDetailsAction.java
@@ -105,7 +105,7 @@ public class CobblerSnippetDetailsAction extends RhnAction {
context.getLoggedInUser().getOrg()));
}
else {
- CobblerSnippet snip = getSnippet(request);
+ CobblerSnippet snip = getSnippet(request, isCreateMode(request));
setupSnippet(request, form, snip);
}
}
@@ -116,12 +116,20 @@ public class CobblerSnippetDetailsAction extends
RhnAction {
* if the set up complains... Also it gets info from the request object
* so this is the right place...
* @param request the request
+ * @param createMode true if this snippet was just created..
* @return the cobbler snippet parameter "name"
*/
- public static CobblerSnippet getSnippet(HttpServletRequest request) {
+ public static CobblerSnippet getSnippet(HttpServletRequest request,
+ boolean createMode) {
createMode? hrm that smells fishy
Same action is used for both creating a snippet and loading an existing
snippet.. Will think on how to fix this..
RequestContext context = new RequestContext(request);
try {
- String name = context.getParam(NAME, true);
+ String name;
+ if (!createMode &&
RhnValidationHelper.getFailedValidation(request)) {
+ name = context.getParam(OLD_NAME, true);
+ }
+ else {
+ name = context.getParam(NAME, true);
+ }
return CobblerSnippet.loadEditable(name,
context.getLoggedInUser().getOrg());
}
@@ -153,9 +161,13 @@ public class CobblerSnippetDetailsAction extends RhnAction
{
private CobblerSnippet submit(HttpServletRequest request, DynaActionForm
form) {
RequestContext context = new RequestContext(request);
+ String name = isCreateMode(request) ?
+ form.getString(NAME) : form.getString(OLD_NAME);
+
+
CobblerSnippet snip = CobblerSnippet.createOrUpdate(
isCreateMode(request),
- form.getString(NAME),
+ name,
form.getString(CONTENTS),
context.getLoggedInUser().getOrg());
if (!isCreateMode(request) &&
diff --git
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetListSetupAction.java
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetListSetupAction.java
index a48daf8..cc81940 100644
---
a/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetListSetupAction.java
+++
b/java/code/src/com/redhat/rhn/frontend/action/kickstart/cobbler/CobblerSnippetListSetupAction.java
@@ -14,6 +14,7 @@
*/
package com.redhat.rhn.frontend.action.kickstart.cobbler;
+import com.redhat.rhn.common.util.DynamicComparator;
import com.redhat.rhn.domain.kickstart.cobbler.CobblerSnippet;
import com.redhat.rhn.domain.user.User;
import com.redhat.rhn.frontend.action.common.BadParameterException;
@@ -25,6 +26,7 @@ import org.apache.struts.action.ActionForm;
import org.apache.struts.action.ActionForward;
import org.apache.struts.action.ActionMapping;
+import java.util.Collections;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
@@ -39,6 +41,8 @@ public class CobblerSnippetListSetupAction extends RhnAction {
private static final String CUSTOM = "custom";
private static final String ALL = "all";
+ private static final DynamicComparator NAME_COMPARATOR =
+ new DynamicComparator("name", true);
Isn't there something in BeanUtils that can do this type of comparator? If you
already looked there, then my apologies.
The does exist a bean comparator, but not in the version of bean utils
we have ..
http://commons.apache.org/beanutils/commons-beanutils-1.7.0/docs/bean-collections/org/apache/commons/beanutils/BeanComparator.html
Again as I mentioned above this class DynamicComparator has been in use
for a couple of year in our list tag. I didn't want to invent a new way
to do this..
/**
* $...@inheritdoc}
*/
@@ -62,7 +66,7 @@ public class CobblerSnippetListSetupAction extends RhnAction {
throw new BadParameterException("Invalid mapping parameter passed!!
[" +
mapping.getParameter() +
"]");
}
-
+ Collections.sort(result, NAME_COMPARATOR);
request.setAttribute("pageList", result);
request.setAttribute("parentUrl", request.getRequestURI());
return mapping.findForward("default");
diff --git
a/java/code/src/com/redhat/rhn/frontend/strings/database/StringResource_en_US.xml
b/java/code/src/com/redhat/rhn/frontend/strings/database/StringResource_en_US.xml
index 58436e5..660ebba 100644
---
a/java/code/src/com/redhat/rhn/frontend/strings/database/StringResource_en_US.xml
+++
b/java/code/src/com/redhat/rhn/frontend/strings/database/StringResource_en_US.xml
@@ -2893,14 +2893,7 @@
http://www.oasis-open.org/committees/xliff/documents/xliff-core-1.1.xsd";
<context-group name="ctx">
<context
context-type="sourcefile">/rhn/systems/details/probes/ProbeDetails.do</context>
</context-group>
- </trans-unit>
-
- <trans-unit id="WARNING">
-<source>WARNING</source>
- <context-group name="ctx">
- <context
context-type="sourcefile">/rhn/systems/details/probes/ProbeDetails.do</context>
- </context-group>
- </trans-unit>
+ </trans-unit>
Did you intend to remove the WARNING above? seems way out of context for
this commit.
The rest of the string cleanup looked good.
There were servel duplicate keys called Warning. But I think I removed
the wrong one.. Will fix this .. Thanks..
diff --git a/java/code/src/com/redhat/rhn/frontend/taglibs/NoteTag.java
b/java/code/src/com/redhat/rhn/frontend/taglibs/NoteTag.java
new file mode 100644
index 0000000..ddd4788
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/frontend/taglibs/NoteTag.java
@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) 2009 Red Hat, Inc.
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.frontend.taglibs;
+
+
+/**
+ * NoteTag
+ * @version $Rev$
+ */
+public class NoteTag extends ToolTipTag {
+
+ /**
+ * Comment for <code>serialVersionUID</code>
+ */
+ private static final long serialVersionUID = -7743415103174952344L;
+
+ @Override
+ protected String geTypeKey() {
geTypeKey? Did you mean getTypeKey?
+ return "Note";
+ }
+}
What's this for? some better javadoc would be useful and where is the unit test?
Why can't folks use <rhn:tooltip type="NOTE" ... > instead of using another tag?
I was kinda confused myself on the better approach
<rhn:note key ="...."/>
vs
<rhn:tooltip key ="...." type = "Note"/>
thing is I think 'Warning:' , 'Tip:' and 'Note:' IMHO covers all the
cases we'd ever want to use.. so why the extra text :)...
I am ok with a vote on this...
diff --git a/java/code/src/com/redhat/rhn/frontend/taglibs/ToolTipTag.java
b/java/code/src/com/redhat/rhn/frontend/taglibs/ToolTipTag.java
index 4a4e284..9190e57 100644
--- a/java/code/src/com/redhat/rhn/frontend/taglibs/ToolTipTag.java
+++ b/java/code/src/com/redhat/rhn/frontend/taglibs/ToolTipTag.java
@@ -39,7 +39,7 @@ public class ToolTipTag extends TagSupport {
private static final long serialVersionUID = 7202597580376186072L;
private String key;
- private String text;
+ private String typeKey = "Tip";
Should these be constants? If not why not just add a default ctor
public ToolTipTag() {
key = null;
text = null;
typeKey = "Tip";
}
I know we do this in other places, just wondering if you meant it to
be a constant
or not.
Well if you do a <rhn:tooltip, it uses the above class..
Me is confused with the question " Should these be constants? "
I mean they being set when the class is instantiated anyway.. I don;t
see difference in them being in the constructor vs being set when they
are member variables..
/**
* Sets the key
@@ -50,11 +50,11 @@ public class ToolTipTag extends TagSupport {
}
/**
- * Sets the text to render...
- * @param txt the text..
+ * Sets the type to render, example "Tip", "Warning", "Note"...
+ * @param typeKeyIn the String resources key of the type..
*/
- public void setText(String txt) {
- text = txt;
+ public void setTypeKey(String typeKeyIn) {
+ typeKey = typeKeyIn;
}
/**
@@ -68,7 +68,7 @@ public class ToolTipTag extends TagSupport {
JspWriter writer = pageContext.getOut();
try {
- writer.write("<span class=\"small-text\">");
+ writer.write("<p class=\"small-text\">");
should the class be a parameter to the tag? or do we always want it to
be small-text?
Yes thats the intent of the tooltip tag..
writer.write(strong.render());
if (!StringUtils.isBlank(key)) {
writer.write(ls.getMessage(key));
@@ -82,7 +82,10 @@ public class ToolTipTag extends TagSupport {
}
protected String geTypeKey() {
- return "Tip";
+ if (StringUtils.isBlank(typeKey)) {
+ return "Tip";
+ }
+ return typeKey;
I think the setTypeKey should check the incoming value and set it to
Tip if it is
blank. Also what if I set it to 'feefifoefum' what will this tag do?
unit test for this class?
***feefifoefum***
May be I 'll just remove type of tool tip so you are forced to use
rhn:tooltip or rhn:warning or rhn:note and not add your own..
}
/**
@@ -91,11 +94,20 @@ public class ToolTipTag extends TagSupport {
public int doEndTag() throws JspException {
JspWriter writer = pageContext.getOut();
try {
- writer.write("</span>");
+ writer.write("</p>");
}
catch (IOException e) {
throw new JspException(e);
}
return SKIP_BODY;
}
+
+ /**
+ * {...@inheritdoc}
+ */
+ public void release() {
+ typeKey = "Tip";
+ key = null;
+ super.release();
+ }
}
diff --git a/java/code/src/com/redhat/rhn/frontend/taglibs/rhn-taglib.tld
b/java/code/src/com/redhat/rhn/frontend/taglibs/rhn-taglib.tld
index efe1d05..9a07c1b 100644
--- a/java/code/src/com/redhat/rhn/frontend/taglibs/rhn-taglib.tld
+++ b/java/code/src/com/redhat/rhn/frontend/taglibs/rhn-taglib.tld
@@ -88,6 +88,11 @@
<required>false</required>
<rtexprvalue>true</rtexprvalue>
</attribute>
+ <attribute>
+ <name>typeKey</name>
+ <required>false</required>
+ <rtexprvalue>true</rtexprvalue>
+ </attribute>
</tag>
<tag>
@@ -101,6 +106,17 @@
</attribute>
</tag>
+ <tag>
+ <name>note</name>
+ <tag-class>com.redhat.rhn.frontend.taglibs.NoteTag</tag-class>
+ <body-content>JSP</body-content>
+ <attribute>
+ <name>key</name>
+ <required>false</required>
+ <rtexprvalue>true</rtexprvalue>
+ </attribute>
+ </tag>
+
<tag>
<name>noscript</name>
diff --git a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/deleteconfirm.jsp
b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/deleteconfirm.jsp
index 91931ce..7b3b52d 100644
--- a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/deleteconfirm.jsp
+++ b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/deleteconfirm.jsp
@@ -27,7 +27,7 @@
<bean:message key="snippetcreate.jsp.contents"/>
</th>
<td>
- <pre style="overflow: scroll; width: 600px; height:
200px">${contents}</pre>
+ <pre class="file-display">${contents}</pre>
</td>
</tr>
</table>
diff --git
a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetdetails.jsp
b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetdetails.jsp
index 793a15d..61e7c7c 100644
--- a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetdetails.jsp
+++ b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetdetails.jsp
@@ -20,6 +20,7 @@
<rhn:toolbar base="h1" img="/img/rhn-icon-info.gif" imgAlt="info.alt.img">
<bean:message key="snippetcreate.jsp.toolbar"/>
</rhn:toolbar>
+<h2><bean:message key="snippetcreate.jsp.header2"/></h2>
</c:when>
<c:otherwise>
<rhn:toolbar base="h1" img="/img/rhn-icon-info.gif" imgAlt="info.alt.img"
@@ -27,10 +28,11 @@
deletionType="snippets">
<c:out value="${requestScope.snippet.displayName}"/>
</rhn:toolbar>
+<h2><bean:message key="snippetdetails.jsp.header2"/></h2>
</c:otherwise>
</c:choose>
-<h2><bean:message key="snippetcreate.jsp.header2"/></h2>
+
<c:choose>
<c:when test="${empty requestScope.create_mode}">
<c:set var="url" value ="/kickstart/cobbler/CobblerSnippetEdit"/>
@@ -50,8 +52,8 @@
<bean:message key="cobbler.snippet.name"/><span
class="required-form-field">*</span>
</th>
<td>
- <html:text property="name"/><br/>
- <rhn:tooltip key="snippetcreate.jsp.tip1"/><br/>
+ <html:text property="name"/>
+ <rhn:tooltip key="snippetcreate.jsp.tip1"/>
<c:if test = "${empty requestScope.create_mode}">
<rhn:warning key="snippetcreate.jsp.warning.tip"/>
<html:hidden property="oldName"/>
@@ -64,7 +66,7 @@
<bean:message key="cobbler.snippet.path"/>:
</th>
<td>
- <c:out value="${requestScope.snippet.displayPath}"/>
<br/>
+ <c:out
value="${requestScope.snippet.displayPath}"/><br/>
<rhn:tooltip key="cobbler.snippet.path.tip"/>
</td>
</tr>
@@ -73,7 +75,7 @@
<bean:message key="cobbler.snippet.macro"/>:
</th>
<td>
- <c:out value="${requestScope.snippet.fragment}"/> <br/>
+ <p><c:out value="${requestScope.snippet.fragment}"/></p>
<rhn:tooltip
key="cobbler.snippet.copy-paste-snippet-tip"/>
</td>
</tr>
@@ -94,7 +96,7 @@
<table class="details">
<tr>
<th>
- <bean:message key="snippetcreate.jsp.contents"/>
+ <bean:message key="snippetcreate.jsp.contents"/><span
class="required-form-field">*</span>
</th>
<td>
<html:textarea property="contents" rows="24" cols="80"
styleId="contents"/>
@@ -107,7 +109,14 @@
<table align="right">
<tr>
<td></td>
- <td align="right"><html:submit><bean:message
key="snippetcreate.jsp.submit"/></html:submit></td>
+ <c:choose>
+ <c:when test = "${empty requestScope.create_mode}">
+ <td align="right"><html:submit><bean:message
key="snippetupdate.jsp.submit"/></html:submit></td>
+ </c:when>
+ <c:otherwise>
+ <td align="right"><html:submit><bean:message
key="snippetcreate.jsp.submit"/></html:submit></td>
+ </c:otherwise>
+ </c:choose>
</tr>
</table>
great reuse of a page :)
diff --git a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippets.jsp
b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippets.jsp
index 27103d9..2e43807 100644
--- a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippets.jsp
+++ b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippets.jsp
@@ -20,13 +20,12 @@
</rhn:toolbar>
<rhn:dialogmenu mindepth="0" maxdepth="1"
definition="/WEB-INF/nav/snippet_tabs.xml"
renderer="com.redhat.rhn.frontend.nav.DialognavRenderer" />
-
+<div class="page-summary">
<p><bean:message key="snippets.jsp.summary"/></p>
-
<c:if test="${not empty requestScope.default}">
- <p>Note:<bean:message key="snippets.jsp.note.default"/></p>
+ <rhn:note key = "snippets.jsp.note.default"/>
I would've expected <rhn:tooltip key="snippets.jsp.note.default" type="note"/>
That way tooltip can be more generic and we can change the look & feel without
having to update the Tag classes, just changing the jsps
</c:if>
-
+</div>
<rl:listset name="keySet">
diff --git a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetview.jsp
b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetview.jsp
index 82572d0..7395a98 100644
--- a/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetview.jsp
+++ b/java/code/webapp/WEB-INF/pages/kickstart/cobbler/snippetview.jsp
@@ -19,7 +19,7 @@
${requestScope.snippet.displayName}
</rhn:toolbar>
-<h2><bean:message key="snippetcreate.jsp.header2"/></h2>
+<h2><bean:message key="snippetdetails.jsp.header2"/></h2>
<div>
<table class="details">
@@ -57,10 +57,10 @@
<table class="details">
<tr>
<th>
- <bean:message key="snippetcreate.jsp.contents"/><span
class="required-form-field">*</span>
+ <bean:message key="snippetcreate.jsp.contents"/>
</th>
<td>
- <pre style="overflow: scroll; width: 600px; height:
200px">${data}</pre>
+ <pre class="file-display">${data}</pre>
</td>
</tr>
</table>
diff --git a/java/code/webapp/WEB-INF/struts-config.xml
b/java/code/webapp/WEB-INF/struts-config.xml
index aa68d39..fcbe318 100644
--- a/java/code/webapp/WEB-INF/struts-config.xml
+++ b/java/code/webapp/WEB-INF/struts-config.xml
@@ -4955,8 +4955,8 @@
<set-property property="acls" value="org_entitlement(rhn_provisioning);
user_role(config_admin)"/>
<forward name="default"
path="/WEB-INF/pages/kickstart/cobbler/deleteconfirm.jsp"/>
- <forward name="success"
- path="/kickstart/cobbler/CobblerSnippetList.do"/>
+ <forward name="success" redirect = "true"
+ path="/kickstart/cobbler/CustomSnippetList.do"/>
Yes redirect="true" is very good :)
WHEW! that was one long commit :)
jesus
Thanks for reviewing.. Will add the mods as suggested :)
_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel
