+1. always good practice.I hope it's OK for a non-committer to start a vote. I know this has been a contentious issue, so I would like to clearly outline my plan for making the html taglibs more extensible and I would like a vote before I go to all the trouble of coding, updating documentation, resolving bugfixes, answering questions, etc.
DESIRED FUNCTIONALITY The ability to cleanly extend the Struts tags. The tags are so good that when an application-specific requirement arises, it's much more desirable to extend from the Struts tag and keep tie-ins with ActionForms and such than it is to go off on your own.
PROPOSED SOLUTION The solution is two parts. Please vote separately for each part of the solution. A) Instead of accessing instance variables directly, use getters.
<snip from="BaseFieldTag.java"> if (accept != null) { results.append(" accept=\""); //old way //results.append(accept); //new way results.append(getAccept()); results.append("\""); } </snip>
If someone wanted to override the accept attribute so that it was always equal to foo then they would be able to do so. A better use case would be overriding the onclick method so that it does something special like display a calendar popup. Getters are actually already used in some places (e.g. - prepareMouseEvents in BaseHandlerTag), so really this is just doing an audit on the code to ensure getters are used consistently throughout the taglib.
Use case for Part A:
I built an implementation
of Matt Kruse's JavaScript calendar widget based on the Struts tags a few
weeks before Matt made his own implementation, so I have some experience
doing this. As some brief background, the widget is a text box and a
corresponding calendar icon. When you click on the calendar icon a popup
appears where you can choose the date. When you click on the text box
(which is what overrides a Struts tag) you want onfocus to automatically
call this.blur() so that the user can't enter text into the textbox (that's
the calendar popup's job). So in my subclass it would be nice to override
the getOnfocus() method instead of overriding the entire
renderIForgetWhatItIsCalled() method. Of course in this particular instance
getOnfocus() is already being used instead of onfocus being accessed
directly, but I think this behavior should be consistent for all attributes.
No solid use case will allways get my -1. I don't want to see unnecessary methods added just becauseB) Add a new renderExtraAttributes() method that gives people the chance to throw non-standard HTML into their tags that extend from Struts tags.
<snip from="BaseFieldTag.java"> results.append("\""); results.append(this.prepareEventHandlers()); results.append(this.prepareStyles()); results.append(this.getElementClose());
<matts-idea> results.append(renderExtraAttributes()); </matt-sidea>
return results.toString(); </snip>
Use Case for Part B:
Unfortunately I still can't think of a good HTML 4.01 compliant use case for
renderExtraAttributes(), but here is a weak try at it.
they might somehow prove useful. I will vote for adding a method that will be used for a specific purpose.
I like the idea of this hook, and I don't personally care if it is valid HTML 4.01 is produced or not,
that is YOUR choice! Because if you NEED to do this you're going to do it anyway, just not as easily.
Also when you say invalid HTML 4.01 do you mean specific to say IE 6.0 or do you mean just adding additional HTML which isn't really invalid?
If my other suggestion of having the render() method call getters instead of directly accessing instance variables is used, then renderExtraAttributes() becomes more important. If it is not provided and someone wants to stick in some non-HTML 4.01 compliant HTML, what they will do is override something like the getSize() method so that it correctly renders the size and then sticks in the understandard HTML. This is a nasty hack but you know a programmer will choose that over duplicating an entire render() method.
If you made it this far, thanks for reading this long email ;) I know I've posted much of this before in several different messages, but hopefully this consolidation is useful.
Matt
--------------------------------------------------------------------- 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]
