myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Bischoff <jbisch...@klkurz.com>
Subject Re: Proposed changes to t:dataTable regarding the new EL specification
Date Wed, 28 Feb 2007 18:33:49 GMT
JSFAttr class has the following attributes incompatible with Facelets:

     // dataTable (extended) attributes
     String ROW_ID                      = 
"org.apache.myfaces.dataTable.ROW_ID";
     String ROW_STYLECLASS_ATTR         = 
"org.apache.myfaces.dataTable.ROW_STYLECLASS";
     String ROW_STYLE_ATTR              = 
"org.apache.myfaces.dataTable.ROW_STYLE";

I will definately fix StyleClass and Style in this patch. Not sure what 
to do about ROW_ID. I don't use it in my app, and not sure what it's 
supposed to do or how to test it. Documentation on the attribute is 
incomplete (literally, half a sentence in the TLD). I see the attribute 
being set in the JSP tag handler, but I don't see this attribute used in 
the extended dataTable class anywhere. Therefore, my feeling is to just 
leave it and fix the other two. Thoughts?

Regards,

Jeff Bischoff
Kenneth L Kurz & Associates, Inc.

Jeff Bischoff wrote:
> If there's no objections, I'll open a JIRA and submit a patch. :)
> 
> Mike Kienenberger wrote:
>> This change looks good to me.  It's probably more important that the
>> value binding be able to default to null than to output an empty
>> string css style.
>>
>>
>> On 2/22/07, Jeff Bischoff <jbischoff@klkurz.com> wrote:
>>> Greets,
>>>
>>> After converting my application from JSP to Facelets, I set out to make
>>> my "rowStyleClass" attribute on t:dataTable work like it used to.
>>>
>>> First, I had to get the attribute working in Facelets. With considerable
>>> discussion on the user list (see [1] and [2]) and a lot of help from
>>> Mike, I think we've identified some pretty simple code changes to enable
>>> this and other similar attributes. I plan to introduce a patch for this,
>>> probably tomorrow.
>>>
>>> What happened next was that during testing of this change, I could
>>> confirm that the attribute did indeed now work, but I was baffled by
>>> unexpected behaviour. My EL expression which had previously returned
>>> null in certain situations was now returning the empty String. I went to
>>> Facelets list for some clarification on this (see [3]) and it turned out
>>> to be a requirement of the new EL spec to coerce the nulls into empty
>>> string.
>>>
>>> Getting an empty String instead of null for this ValueBinding lookup
>>> creates a problem because the extended dataTable treats a null value
>>> differently and goes looking at the more standard style attributes like
>>> rowClasses. With an empty string returned, it assumes it needs to look
>>> no further. As a user, there is no way for me to specify that under
>>> certain conditions, it should fallback to the other style attributes,
>>> e.g. rowClasses.
>>>
>>> The best fix we have collectively come up with so far is to coerce the
>>> empty string back into a null in the dataTable, so that the renderer
>>> does the right thing. I don't see too much downside to this approach, as
>>> an empty style string has no relevance in CSS. However, I feel a
>>> proposed change like this requires extra discussion before making a
>>> decision on it. It's another one of those situation where we have to
>>> decide how we want to handle unexpected results due to changes in the
>>> newer specs.
>>>
>>> If you review the threads a bit, you can see more of the details of the
>>> issues. I'll post my preliminary proposed code changes at the bottom.
>>>
>>> [1] https://facelets.dev.java.net/servlets/ReadMsg?list=users&msgNo=6875
>>> [2]
>>> http://www.nabble.com/Re%3A-Facelets-support-for-a-Tomahawk-dataTable-trick--tf3236491.html

>>>
>>> [3] https://facelets.dev.java.net/servlets/ReadMsg?list=users&msgNo=6941
>>>
>>> Regards,
>>>
>>> Jeff Bischoff
>>> Kenneth L Kurz & Associates, Inc.
>>>
>>>
>>> Okay proposed code change in
>>> org.apache.myfaces.component.html.ext.HtmlDataTable
>>>
>>> Original Method:
>>>
>>>      public String getRowStyleClass()
>>>      {
>>>          if (_rowStyleClass != null)
>>>              return _rowStyleClass;
>>>          ValueBinding vb = getValueBinding(JSFAttr.ROW_STYLECLASS_ATTR);
>>>          return vb != null ? (String) vb.getValue(getFacesContext()) 
>>> : null;
>>>      }
>>>
>>>
>>> New Method:
>>>
>>>      public String getRowStyleClass()
>>>      {
>>>      if (_rowStyleClass != null)
>>>          return _rowStyleClass;
>>>
>>>      // TODO: temporarily support fully-qualified ext. dataTable
>>> attribute names.
>>>      ValueBinding vb =
>>> getValueBinding("org.apache.myfaces.dataTable.ROW_STYLECLASS");
>>>      if (vb != null)
>>>          log.warn("org.apache.myfaces.dataTable.ROW_STYLECLASS is
>>> deprecated. Please use rowStyleClass instead.");
>>>      else
>>>          vb = getValueBinding(JSFAttr.ROW_STYLECLASS_ATTR);
>>>      if(vb == null)
>>>          return null;
>>>      String bindingValue = (String) vb.getValue(getFacesContext());
>>>      if(bindingValue == "")
>>>          return null;  // Fix for JSF 1.2 EL coercing nulls to empty 
>>> string
>>>      return bindingValue;
>>>      }
>>>
>>> That, along with the change to JSFAttr to change the constant value from
>>> "org.apache.myfaces.dataTable.ROW_STYLECLASS" to "rowStyleClass". NOTE:
>>> This change affects the shared code!
>>>
>>>
>>>
>>
>>
>>
> 
> 
> 
> 
> 



Mime
View raw message