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 21:21:02 GMT
Hi Mike,

Sorry! I forgot about TOMAHAWK-523. I remembered there were the similar 
JIRA issues that affected Tree2, but didn't remember this one existed. I 
see you already marked it as part of TOMAHAWK-914, thank you!!

Regards,

Jeff Bischoff
Kenneth L Kurz & Associates, Inc.

Mike Kienenberger wrote:
> Jeff,
> 
> In the future, it would be better to add patches to an existing JIRA
> issue when one exists rather than opening a new one.
> 
> On 2/28/07, Jeff Bischoff <jbischoff@klkurz.com> wrote:
>> 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