3:58 a.m.
Author: maksimkaszynski
Date: 2008-04-10 04:58:03 -0400 (Thu, 10 Apr 2008)
New Revision: 7726
Modified:
trunk/ui/inplaceInput/src/main/java/org/richfaces/renderkit/InplaceInputBaseRenderer.java
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/css/inplaceinput.xcss
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/scripts/inplaceinput.js
Log:
code reviewed
Modified:
trunk/ui/inplaceInput/src/main/java/org/richfaces/renderkit/InplaceInputBaseRenderer.java
===================================================================
---
trunk/ui/inplaceInput/src/main/java/org/richfaces/renderkit/InplaceInputBaseRenderer.java 2008-04-10
08:42:40 UTC (rev 7725)
+++
trunk/ui/inplaceInput/src/main/java/org/richfaces/renderkit/InplaceInputBaseRenderer.java 2008-04-10
08:58:03 UTC (rev 7726)
@@ -76,20 +76,28 @@
return;
}
+ //TODO:
+ //First check NULL clientId
+
if (InputUtils.isDisabled(inplaceInput) || InputUtils.isReadOnly(inplaceInput)) {
if (logger.isDebugEnabled()) {
logger.debug(("No decoding necessary since the component "
+ component.getId() + " is disabled"));
}
+ //TODO: return?
}
String clientId = component.getClientId(context);
+ //TODO: NULL is NULL?
if (clientId == null) {
throw new NullPointerException("component " +
inplaceInput.getClientId(context) + " client id is NULL" );
}
-
+ //TODO:
+ //Should use clientId instead?
clientId = clientId + UIInplaceInput.VALUE_SUFFIX;
Map request = context.getExternalContext().getRequestParameterMap();
+ //TODO:
+ //Request.get is enough
if (request.containsKey(clientId)) {
String newValue = (String)request.get(clientId);
if (newValue != null) {
@@ -98,6 +106,8 @@
}
}
+ //TODO:
+ //Move that code to template, maybe?
public void encodeControlsFacet(FacesContext context, UIComponent component) throws
IOException {
UIComponent facet = component.getFacet(CONTROLS_FACET);
if ((facet != null) && (facet.isRendered())) {
@@ -114,12 +124,15 @@
}
public String encodeScriptAttributes(FacesContext context, UIComponent component) {
+ //TODO:
+ //Use string builder instead
StringBuffer attributes = new StringBuffer();
attributes.append("var attributes = ");
ScriptOptions options = new ScriptOptions(component);
String defaultLabel =
(String)component.getAttributes().get("defaultLabel");
+ //TODO: length()==0
if (defaultLabel == null || defaultLabel.equals("")) {
defaultLabel = EMPTY_DEFAULT_LABEL;
}
@@ -176,6 +189,7 @@
return cssMap.toString();
}
+ //TODO: use enum?
private String buildCss(UIComponent component, int key, String suffix) {
Object value;
StringBuffer stringBuffer = new StringBuffer();
@@ -214,6 +228,7 @@
return stringBuffer.toString();
}
+ //TODO: remove?
public String getAsEventHandler(FacesContext context, UIComponent component, String
attributeName) {
String event = (String) component.getAttributes().get(attributeName);
String result = null;
@@ -242,6 +257,7 @@
}
protected boolean isEmptyDefaultLabel(String defaultLabel) {
+ //TODO: remove != null?
if (defaultLabel != null && EMPTY_DEFAULT_LABEL.equals(defaultLabel)) {
return true;
}
Modified:
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/css/inplaceinput.xcss
===================================================================
---
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/css/inplaceinput.xcss 2008-04-10
08:42:40 UTC (rev 7725)
+++
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/css/inplaceinput.xcss 2008-04-10
08:58:03 UTC (rev 7726)
@@ -7,6 +7,11 @@
<f:verbatim>
<![CDATA[
+ /***
+ TODO: check for duplicates!!
+ ***/
+
+
.rich-inplace {
/*position:relative;*/
}
Modified:
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/scripts/inplaceinput.js
===================================================================
---
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/scripts/inplaceinput.js 2008-04-10
08:42:40 UTC (rev 7725)
+++
trunk/ui/inplaceInput/src/main/resources/org/richfaces/renderkit/html/scripts/inplaceinput.js 2008-04-10
08:58:03 UTC (rev 7726)
@@ -3,7 +3,7 @@
Richfaces.InplaceInput.prototype = {
-
+ //TODO: remove $$$$$
initialize: function(clientId, temValueKeepId, valueKeepId, tabberId, attributes,
events, classes, barParams) {
this.inplaceInput = $(clientId);
this.inplaceInput.component = this;
@@ -15,7 +15,7 @@
this.events = events;
this.classes = classes;
-
+ //TODO: static methods are preferred to be called within constructor
this.currentText = this.getCurrentText();
this.value = this.valueKeeper.value;
@@ -23,9 +23,11 @@
this.prevState = Richfaces.InplaceInput.STATES[0];
if (this.attributes.showControls) {
+ //TODO: Consider passing attributes by name instead of by index
this.bar = new Richfaces.InplaceInputBar(barParams[0], barParams[1], barParams[2],
barParams[3], barParams[4],
this.attributes.verticalPosition, this.attributes.horizontalPosition);
}
+ //TODO: move converting 'on'-event to prototype style to utils
this.editEvent =
this.attributes.editEvent.substring(2,this.attributes.editEvent.length);
@@ -37,16 +39,17 @@
},
+ //TODO: more cleanup here - remove references to DOM elements
destroy: function() {
this.inplaceInput.component = null;
},
initHandlers : function() {
- this.inplaceInput.observe(this.editEvent,
function(e){this.switchingStatesHandler(e);}.bindAsEventListener(this));
- this.inplaceInput.observe("mouseout",
function(e){this.inplaceMouseOutHandler(e);}.bindAsEventListener(this));
- this.inplaceInput.observe("mouseover",
function(e){this.inplaceMouseOverHandler(e);}.bindAsEventListener(this));
+ this.inplaceInput.observe(this.editEvent,
this.switchingStatesHandler.bindAsEventListener(this));
+ this.inplaceInput.observe("mouseout",
this.inplaceMouseOutHandler.bindAsEventListener(this));
+ this.inplaceInput.observe("mouseover",
this.inplaceMouseOverHandler.bindAsEventListener(this));
-
+ //TODO: do like above - no need to create extra functions
this.tempValueKeeper.observe("blur",
function(e){this.tmpValueBlurHandler(e);}.bindAsEventListener(this));
this.tempValueKeeper.observe("keydown",
function(e){this.tmpValueKeyDownHandler(e);}.bindAsEventListener(this));
@@ -79,6 +82,7 @@
*/
inplaceMouseOverHandler : function(e) {
+ //TODO: Nick recommends regexp
var className = this.inplaceInput.className;
if (this.currentState == Richfaces.InplaceInput.STATES[0]) {
if (className.indexOf(this.classes.COMPONENT.VIEW.HOVERED) == -1) {
@@ -100,6 +104,7 @@
}
},
+ //TODO: Event.element here?
switchingStatesHandler : function(e) {
var el = (e.srcElement) ? e.srcElement : e.target;
if ((el.id == this.inplaceInput.id) || (e.type == "focus")) {
@@ -120,6 +125,8 @@
}
}
+//DONT TUCH THIZ CODE
+
if (this.invokeEvent(this.events.oneditactivation, this.inplaceInput,
"rich:oneditactivation", {oldValue : this.valueKeeper.value, value :
this.tempValueKeeper.value})) {
var textSize = this.inplaceInput.offsetWidth;
@@ -210,6 +217,7 @@
}
if (this.attributes.selectOnEdit) {
+ //TODO: implement Select all method?
Richfaces.InplaceInput.textboxSelect(this.tempValueKeeper, 0,
this.tempValueKeeper.value.length);
}
this.tempValueKeeper.focus();
@@ -221,6 +229,7 @@
this.createNewText(this.currentText);
this.inplaceInput.className = this.classes.COMPONENT.VIEW.NORMAL;
+ //TODO: see above
this.inplaceInput.observe("mouseover",
function(e){this.inplaceMouseOverHandler(e);}.bindAsEventListener(this));
},
@@ -238,7 +247,9 @@
* UTILITIES
*/
+ //TODO: rename parameter to textWidth?
setInputWidth : function(textSize) {
+ //TODO: use constants here
if (this.currentState != 1) {
return;
}
@@ -335,7 +346,8 @@
}
}
},
-
+ //TODO: remove Bucks
+ //TODO: to shared library
invokeEvent : function(eventFunc, element, eventName, memo) {
var result = false;
if (eventFunc) {
@@ -370,7 +382,7 @@
this.inplaceInput.removeChild(text);
}
},
-
+ //TODO: search within childNodes for textNode.value().trim().length == 0?
getCurrentText : function() {
return this.inplaceInput.childNodes[4];
},
@@ -398,6 +410,7 @@
},
initDimensions : function() {
+ //TODO: Crap code magic numbers - I propose 325 and 675
/*this.bar.style.visibility = "hidden";
this.bar.show();*/
@@ -467,6 +480,7 @@
Richfaces.InplaceInput.textboxSelect = function(oTextbox, iStart, iEnd) {
if (Prototype.Browser.IE) {
+ //TODO: dont check 4 browser. Check for capability instead.
var oRange = oTextbox.createTextRange();
oRange.moveStart("character", iStart);
oRange.moveEnd("character", -oTextbox.value.length + iEnd);