Comments in line. ----- Original Message ----- From: "Marcus Breese" <[EMAIL PROTECTED]> To: "Struts Developers List" <[EMAIL PROTECTED]> Sent: Tuesday, June 22, 2004 10:29 PM Subject: Re: PATCH: html:cancel tag. Made a javascript version for submitting
> I completely agree with your statements regarding a javascript > requirement. My only issue is a strict user interface issue. Meaning > that the current implementation goes against user interface > guidelines, specifically that "enter" should submit a form, not cancel > it. I don't agree - its nothing to do with the Struts implementation - its a browser/html issue. > Perhaps the entire thing could be wrapped in a <noscript> tag to > display the default (non-javascript behaviour) in the case that the > user has javascript turned off. > > Would something like that be more acceptable? Not to me - the other points I made about being a non *core* Struts tag and whether on principle we implement people's custom javascript behaviours or not still stand. > I haven't followed the JSTL vs Struts tag debate for a long time, so > I'm not quite sure how they integrate, but then again, there's always > something new to learn... : ) Some of them directly replace Struts tags. > Thanks... No problem, Sorry don't like to be negative. Niall > > On Tue, 22 Jun 2004 22:03:03 +0100, Niall Pemberton > <[EMAIL PROTECTED]> wrote: > > > > You're not out of line at all - this list is the place to discuss > > enhancement requests and requests which come with a patch attached are more > > welcome than those without :-) > > > > Having said that, I'm against this change. If the user disables javascript > > in their browser then the cancel button no longer works. Even with your > > enhanced patch to turn on/off the additional behaviour by setting a new > > javascript attribute to true/false, the fact remains that the cancel button > > would stop working with javascript disabled. I bet we would have questions > > every week titled "Cancel Button Doesn't Work" on the user list and it will > > get boring typing the same "Did you set the javascript='true' attribute etc > > etc." > > > > You might say that the same arguments is true for the existing javascript in > > the CancelTag (i.e. onclick="bCancel=true") but I wouldn't agree with that. > > Although that javascript would also not work with javascript disabled, the > > fact is that it interacts with the javascript rendered by the validator > > framework - that javascript IMO is an optional extra because with javascript > > disabled the validator framework's server side validation would still work > > and the system would not be *broken*. > > > > Another point about the issue you are trying to resolve is that its a > > browser/html isssue and nothing to do with how Struts works. If we put in > > this patch to implement some custom javascript behaviour then what about the > > next person who wants their own funky bit of javascript in Struts to > > implement their custom behaviour? > > > > If you look at the CancelTag it doesn't actually do much "Struts" wise - you > > could easily do what it does directly in html (including the behaviour you > > want), it just saves keystrokes. Maybe Jakarta Taglibs would be a better > > place for this tag and the behaviour that you want - they have an "input" > > tags component. > > > > http://jakarta.apache.org/taglibs/doc/input-doc/intro.html > > > > When Struts was *born* there wasn't much else around - but now many of the > > Struts tags can be replaced with JSTL and the current view (which I agree > > with) seems to be that the tags should be split into *core* Struts tags - > > tags which implement behaviour specific to Struts and the rest which will be > > kept on as "legacy" tags. The idea being that we should only continue to > > develop *core* tags. IMO the cancel tag falls into the later category - > > others may differ though. > > > > Niall > > > > > > > > ----- Original Message ----- > > From: "Marcus Breese" <[EMAIL PROTECTED]> > > To: <[EMAIL PROTECTED]> > > Sent: Tuesday, June 22, 2004 7:22 PM > > Subject: PATCH: html:cancel tag. Made a javascript version for submitting > > > > > I'm submitting this again here because my Bugzilla patch hasn't gone > > > anywhere, so I don't know if anyone likes this idea or not. > > > Basically, if the cancel button is after the submit button on a form, > > > and the user hits 'enter' to submit the form, the the cancel button is > > > the one that is activated, causing the form to be cancelled. So, I > > > added a "javascript" property to the cancel button, that when true > > > causes the javascript button to be rendered as a combination html > > > button and a hidden text field that are linked through a javascript > > > onclick event. It is completely backwards compatible and doesn't > > > break any current pages (that I know of). > > > > > > If I'm completely out of line sending this here, I appologize... > > > > > > Bugzilla link: http://issues.apache.org/bugzilla/show_bug.cgi?id=25860 > > > > > > PATCH > > > ==================== > > > > > > Index: doc/userGuide/struts-html.xml > > > =================================================================== > > > RCS file: /home/cvspublic/jakarta-struts/doc/userGuide/struts-html.xml,v > > > retrieving revision 1.67 > > > diff -u -r1.67 struts-html.xml > > > --- doc/userGuide/struts-html.xml 2 Jan 2004 11:55:38 -0000 1.67 > > > +++ doc/userGuide/struts-html.xml 3 Jan 2004 00:44:06 -0000 > > > @@ -410,6 +410,16 @@ > > > disabled. > > > </info> > > > </attribute> > > > + <attribute> > > > + <name>javascript</name> > > > + <required>false</required> > > > + <rtexprvalue>true</rtexprvalue> > > > + <info> > > > + Set to <code>true</code> if this input field should be > > > + rendered using a javascript button/hidden field combination > > > + as opposed to a submit button. > > > + </info> > > > + </attribute> > > > > > > <attribute> > > > <name>onblur</name> > > > Index: src/share/org/apache/struts/taglib/html/CancelTag.java > > > =================================================================== > > > RCS file: > > /home/cvspublic/jakarta-struts/src/share/org/apache/struts/taglib/html/Cance > > lTag.java,v > > > retrieving revision 1.13 > > > diff -u -r1.13 CancelTag.java > > > --- src/share/org/apache/struts/taglib/html/CancelTag.java 31 Jul 2003 > > > 00:19:04 -0000 1.13 > > > +++ src/share/org/apache/struts/taglib/html/CancelTag.java 3 Jan 2004 > > > 00:44:06 -0000 > > > @@ -106,6 +106,12 @@ > > > */ > > > protected String value = null; > > > > > > + /** > > > + * Should this be rendered using javascript ? > > > + */ > > > + > > > + protected String javascript = null; > > > + > > > > > > // ------------------------------------------------------------- > > Properties > > > > > > @@ -144,6 +150,35 @@ > > > } > > > > > > > > > + > > > + /** > > > + * return the javascript value > > > + */ > > > + public String getJavascript() { > > > + return javascript; > > > + } > > > + > > > + /** > > > + * @param javascript > > > + */ > > > + public void setJavascript(String javascript) { > > > + this.javascript = javascript; > > > + } > > > + > > > + /** > > > + * > > > + * Return a boolean for if this should be rendered using javascript > > > + */ > > > + > > > + public boolean renderJavascript() { > > > + if (javascript!=null && javascript.toUpperCase().equals("TRUE")) { > > > + return true; > > > + } > > > + return false; > > > + } > > > + > > > + > > > + > > > // --------------------------------------------------------- Public > > Methods > > > > > > > > > @@ -192,10 +227,14 @@ > > > > > > // Generate an HTML element > > > StringBuffer results = new StringBuffer(); > > > - results.append("<input type=\"submit\""); > > > - results.append(" name=\""); > > > - results.append(property); > > > - results.append("\""); > > > + if (renderJavascript()) { > > > + results.append("<input type=\"button\""); > > > + } else { > > > + results.append("<input type=\"submit\""); > > > + results.append(" name=\""); > > > + results.append(property); > > > + results.append("\""); > > > + } > > > if (accesskey != null) { > > > results.append(" accesskey=\""); > > > results.append(accesskey); > > > @@ -212,12 +251,33 @@ > > > results.append(prepareEventHandlers()); > > > results.append(prepareStyles()); > > > > > > - // if no onclick event was provided, put in the cancel script > > > - if(results.toString().indexOf("onclick=")==-1){ > > > - results.append(" onclick=\"bCancel=true;\""); > > > - } > > > - > > > - results.append(getElementClose()); > > > + if (renderJavascript()) { > > > + String insertStr = " > > > document.getElementById('"+property+"').value='Cancel'; "; > > > + insertStr += " > > > document.getElementById('"+property+"').name='"+property+"'; "; > > > + insertStr += " this.form.submit(); "; > > > + > > > + // if no onclick event was provided, put in the cancel script > > > + if(results.toString().indexOf("onclick=")==-1){ > > > + results.append(" onclick=\"bCancel=true; "); > > > + results.append(insertStr); > > > + } else { > > > + int onclickPos = results.toString().indexOf("onclick=\""); > > > + int nextQuotePos= results.toString().indexOf("\"",onclickPos+10); > > > + results.insert(nextQuotePos,insertStr); > > > + } > > > + > > > + results.append(getElementClose()); > > > + > > > + results.append("<input type=\"hidden\" id=\""+property+"\" > > value=\"\">"); > > > + } else { > > > + // if no onclick event was provided, put in the cancel script > > > + > > > + if(results.toString().indexOf("onclick=")==-1){ > > > + results.append(" onclick=\"bCancel=true;\" "); > > > + } > > > + results.append(getElementClose()); > > > + > > > + } > > > > > > // Render this element to our writer > > > TagUtils.getInstance().write(pageContext, results.toString()); > > > @@ -233,12 +293,14 @@ > > > */ > > > public void release() { > > > > > > - super.release(); > > > - property = Constants.CANCEL_PROPERTY; > > > + super.release(); > > > + property = Constants.CANCEL_PROPERTY; > > > text = null; > > > - value = null; > > > + value = null; > > > + javascript=null; > > > > > > } > > > > > > > > > } > > > + > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
