Will this have the same null issue with facelets?

ValueBinding vb =
getValueBinding("org.apache.myfaces.dataTable.ROW_STYLECLASS");
if (vb != null) ...

On 2/22/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
Hmm my indentation got a little bit skewed there... let's see if this is
any better.

     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;
     }

Jeff Bischoff wrote:
> Well I've just tested and if I make our dataTable coerce the empty
> string back into a null, everything works splendidly for me. :)
>
> Do you think it is safe to do? My view is that an empty style string can
> have no impact anyway, so the component might as well continue looking
> at the other style attributes. The only possible pitfall I see is
> consistency - people who returned an empty string and expected it *not*
> to fallback to the other attributes. Not exactly sure why anyone would
> do that, but you never know....
>
> 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. See
> any problems? Hmm... why do I feel like I should be posting this to a
> JIRA issue for discussion by this point? Let me see if I can figure out
> how to make some nice diff files for a patch.
>
> Regards,
>
> Jeff Bischoff
> Kenneth L Kurz & Associates, Inc.
>
> Mike Kienenberger wrote:
>> Jeff,
>>
>> My suggestion is to go with empty-string.
>>
>> If I understand correctly, the only time when null would be returned
>> is if you were using JSF 1.1 with jsp, which isn't what you're doing.
>>
>>
>> On 2/22/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
>>> David,
>>>
>>> This does work - as long as you know what CSS you want for the
>>> unselected rows. Unfortunately, rowSytleClass does not support
>>> alternating styles the way the rowClasses attribute does.
>>>
>>> I really wish this solution was adequate for my needs, because it is
>>> looking increasingly likely that my little "trick" of returning null in
>>> the EL is required by the spec *not* to work in JSF 2.0+ EL. :(
>>>
>>> So I'm left scratching my head how to keep my alternating row styles and
>>> also be able to highlight one.
>>>
>>> Regards,
>>>
>>> Jeff Bischoff
>>> Kenneth L Kurz & Associates, Inc.
>>>
>>> Nebinger, David wrote:
>>> > Instead of returning null, could you return something like
>>> "nonHighlightRow"?  This would probably work for both cases (jsp &
>>> facelets) with the only overhead of defining another class...
>>> >
>>> > ________________________________
>>> >
>>> > From: Jeff Bischoff [mailto:[EMAIL PROTECTED]
>>> > Sent: Thu 2/22/2007 11:30 AM
>>> > To: MyFaces Discussion
>>> > Subject: Re: Facelets support for a Tomahawk dataTable trick?
>>> >
>>> >
>>> >
>>> > Well, you were right Mike, in that the testing is taking the
>>> longest. I
>>> > mean, we're really only talking about a few lines of code here. When I
>>> > made the changes you suggested, it did "fix" my code so that the
>>> > appropriate rows had the rowStyleClass applied to them.
>>> >
>>> > Unfortunately, it has caused some other problems apparently due to a
>>> > difference in the way Facelets resolves value-bindings.
>>> >
>>> > When my EL expression is resolved in dataTable class:
>>> >
>>> > #{tableData.selectedRowIndex == rowIndex ? 'highlightRow' : null}
>>> >
>>> > If the condition is true, then it correctly returns the String
>>> > "highlightRow" in both JSP and Facelets. If the condition is false in
>>> > JSP, it returns null for the value binding's value. If the
>>> condition is
>>> > false in facelets, it instead returns the empty String. This is a
>>> > problem because the renderer then thinks that there is a non-null
>>> > rowStyleClass and tries to apply it. This means that no CSS gets
>>> applied
>>> > to those rows at all, while I would expect them to get the default CSS
>>> > defined by rowClasses attribute.
>>> >
>>> > So I either need to figure out why Facelets is returning the empty
>>> > string here, or just change the renderer to check for null OR the
>>> empty
>>> > string; i.e. treat the empty string as if it were null. Is that an
>>> > acceptable approach? I guess it would be better to figure out why this
>>> > is different in the first place.
>>> >
>>> > Regards,
>>> >
>>> > Jeff Bischoff
>>> > Kenneth L Kurz & Associates, Inc.
>>> >
>>> > Jeff Bischoff wrote:
>>> >> Alright Mike, I'll give it a shot.
>>> >>
>>> >> Mike Kienenberger wrote:
>>> >>> No, you can fix the jsp tag handler (and/or JSFAttr constants)
>>> without
>>> >>> changing backwards compatibility.
>>> >>>
>>> >>> What would break is facelets code workarounds like the following.
>>> >>>
>>> >>> <t:dataTable id="TheDataTable"
>>> >>>     ...
>>> >>>     rowClasses="oddRow,evenRow"
>>> >>>
>>> >>>
>>> 
org.apache.myfaces.dataTable.ROW_STYLECLASS="#{dataTableBacking.selectedRowIndex
>>>
>>> >>>
>>> >>> ==  rowIndex ? 'highlightRow' : null}"
>>> >>>     rowIndexVar="rowIndex" .../>
>>> >>>
>>> >>> The fix to insure backward compatibility would be to check if
>>> >>> "org.apache.myfaces.dataTable.ROW_STYLECLASS" was defined as a
>>> generic
>>> >>> attribute and to use that if no "rowStyleClass" value is present.
>>> >>>
>>> >>> So a complete fix is to change JSFAttr.ROW_STYLECLASS to
>>> >>> "rowStyleClass" and then to add in Tomahawk-47-esque backward
>>> >>> compatiblity fallback/warning.
>>> >>>
>>> >>> On 2/21/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
>>> >>>> Ooooooh... I see now how the tag handler is placing the value
>>> there and
>>> >>>> Facelets is not. Its not the renderer itself that expects the
>>> value to
>>> >>>> be there, but the HtmlDataTable class itself. (Which is called
>>> during
>>> >>>> rendering, so equivalent).
>>> >>>>
>>> >>>> Thanks, Mike.
>>> >>>>
>>> >>>> So, in order to fix but still maintain backwards compatibility,
>>> I need
>>> >>>> to have the JSP tag handler place the value in both locations,
>>> right?
>>> >>>> And then have HtmlDataTable check both locations? Your comment on
>>> >>>> TOMAHAWK-47 has code for doing the latter (for showNav). Is that
>>> all I
>>> >>>> need, or both? I don't want to break anything with this "fix".
>>> >>>>
>>> >>>> Regards,
>>> >>>>
>>> >>>> Jeff Bischoff
>>> >>>> Kenneth L Kurz & Associates, Inc.
>>> >>>>
>>> >>>> Mike Kienenberger wrote:
>>> >>>>> Yes, the problem is that JSFAttr.ROW_STYLECLASS_ATTR =
>>> >>>>> "org.apache.myfaces.dataTable.ROW_STYLECLASS" instead of
>>> >>>>> "rowStyleClass"
>>> >>>>>
>>> >>>>> Thus, jsp saves "rowStyleClass" tag values under
>>> >>>>> "org.apache.myfaces.dataTable.ROW_STYLECLASS" while facelets
>>> stores
>>> >>>>> them under "rowStyleClass".
>>> >>>>>
>>> >>>>> http://issues.apache.org/jira/browse/TOMAHAWK-523
>>> >>>>> http://issues.apache.org/jira/browse/TOMAHAWK-35
>>> >>>>>
>>> >>>>> Again, if the tag file were calling the concrete getters and
>>> setters,
>>> >>>>> there'd be no problem.  But it's storing this value as a generic
>>> >>>>> attribute.
>>> >>>>>
>>> >>>>> Note also that your patch will have to provide backward
>>> compatiblity
>>> >>>>> as per my comment at the end of this issue:
>>> >>>>>
>>> >>>>> http://issues.apache.org/jira/browse/TOMAHAWK-47
>>> >>>>>
>>> >>>>> This does show what the workaround is, though.
>>> >>>>> Use this in your page code:
>>> >>>>>
>>> >>>>> <t:dataTable ...
>>> >>>>>
>>> >>>>
>>> org.apache.myfaces.dataTable.ROW_STYLECLASS="#{tableData.selectedRowIndex
>>>
>>> >>>>
>>> >>>>> == rowIndex ? 'highlightRow' : null}" value=....>
>>> >>>>>
>>> >>>>> It'd probably be good to get this in the facelets wiki for all
>>> of the
>>> >>>>> fully-qualified (or oddly-named) attributes listed in
>>> >>>>> org.apache.myfaces.renderkit.JSFAttr.
>>> >>>>>
>>> >>>>> On 2/21/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
>>> >>>>>> I have found the difference in behaviour between Facelets and
>>> JSP to
>>> >>>>>> stem from different results in code from the following class:
>>> >>>>>>
>>> >>>>>> org.apache.myfaces.component.html.ext.HtmlDataTable
>>> >>>>>>
>>> >>>>>> In the method getRowStyleClass(), the following line is called:
>>> >>>>>>
>>> >>>>>> ValueBinding vb = getValueBinding(JSFAttr.ROW_STYLECLASS_ATTR);
>>> >>>>>>
>>> >>>>>>
>>> >>>>>> In JSP, this results in a valuebinding with the String that I set
>>> >>>> (e.g.
>>> >>>>>> #{tableData.selectedRowIndex == rowIndex ? 'highlightRow' :
>>> null}).
>>> >>>>>>
>>> >>>>>> In JSP, this valuebinding correctly returns either
>>> 'highlightRow' or
>>> >>>>>> 'null' as the value.
>>> >>>>>>
>>> >>>>>> In Facelets, this valuebinding is null.
>>> >>>>>>
>>> >>>>>> Okay, why would this valuebinding be null in Facelets? I could
>>> have
>>> >>>>>> understood it failing to properly evaluate the valuebinding
>>> due to
>>> >>>> some
>>> >>>>>> missing variable like "rowIndex", but to have the entire
>>> >>>> expression null?
>>> >>>>>> Anybody got any ideas? I'm guessing this has something to do with
>>> >>>>>> Facelets having a different EL implementation?
>>> >>>>>>
>>> >>>>>> Regards,
>>> >>>>>>
>>> >>>>>> Jeff Bischoff
>>> >>>>>> Kenneth L Kurz & Associates, Inc.
>>> >>>>>>
>>> >>>>>> Jeff Bischoff wrote:
>>> >>>>>>> I'll resume looking at this next week.
>>> >>>>>>>
>>> >>>>>>> I have some clues from putting debugging output into the
>>> source and
>>> >>>>>>> building locally. I need more time to assess it.
>>> >>>>>>>
>>> >>>>>>> So far, I don't see evidence to support your hunch. The tag
>>> handler
>>> >>>>>>> really isn't doing anything special, just a simple:
>>> >>>>>>>
>>> >>>>>>> setStringProperty(component, JSFAttr.ROW_STYLECLASS_ATTR,
>>> >>>>>> _rowStyleClass);
>>> >>>>>>> We'll see, I need to look at it more. I think though, my problem
>>> >>>> may be
>>> >>>>>>> that "rowIndexVar" doesn't get set in time.
>>> >>>>>>>
>>> >>>>>>> Regards,
>>> >>>>>>>
>>> >>>>>>> Jeff Bischoff
>>> >>>>>>> Kenneth L Kurz & Associates, Inc.
>>> >>>>>>>
>>> >>>>>>> Jeff Bischoff wrote:
>>> >>>>>>>> Thanks for the assessment Mike. Fortunately I did just finish
>>> >>>>>>>> converting the last few pages of my application to facelets, so
>>> >>>>>>>> hopefully I'll have time to give it a shot next week. I'm
>>> going to
>>> >>>>>>>> spend the next half hour or so looking into this issue, for
>>> >>>> starters.
>>> >>>>>>>> This is not a huge thing, but it's really the only thing that
>>> >>>> "broke"
>>> >>>>>>>> when switching to facelets. So I'd really like to knock it off.
>>> >>>>>>>>
>>> >>>>>>>> Mike Kienenberger wrote:
>>> >>>>>>>>  > Jeff,
>>> >>>>>>>>  >
>>> >>>>>>>>  > This is really straight-forward stuff.   Two hours should be
>>> >>>>>> more than
>>> >>>>>>>>  > sufficient.
>>> >>>>>>>>  > I'd guess it'd take about 5 minutes to cut&paste a similar
>>> >>>>>>>>  > getter/setter pair, then replace the attribute name with the
>>> >>>> new
>>> >>>>>>>>  > attribute name.
>>> >>>>>>>>  >
>>> >>>>>>>>  > The less-obvious part is going to be going into the renderer
>>> >>>> and
>>> >>>>>>>>  > changing the references to the generic attribute (map entry)
>>> >>>> into a
>>> >>>>>>>>  > concrete method call.
>>> >>>>>>>>  >
>>> >>>>>>>>  > The longest part is going to be testing the changes.
>>> >>>>>>>>  >
>>> >>>>>>>>  > On 2/15/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
>>> >>>>>>>>  >> (I've moved this thread to the myfaces-users list, due to
>>> >>>> it being
>>> >>>>>>>>  >> identified as a Tomahawk bug)
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> Heh, Mike do you ever get tired of answering my
>>> questions? ;)
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> I looked through MyFaces JIRA, and the closest issue I
>>> >>>> found was
>>> >>>>>>>>  >> TOMAHAWK-523. The only difference is that they were
>>> trying to
>>> >>>>>> use EL
>>> >>>>>>>>  >> based off the "var" attribute, whereas I am attempting to
>>> >>>> use the
>>> >>>>>>>>  >> "rowIndexVar". However, this might be the same issue.
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> That issue is marked "patch available", but there are no
>>> files
>>> >>>>>>>> attached.
>>> >>>>>>>>  >> I see that one of your comments on the thread indicates
>>> >>>> that the
>>> >>>>>>>> patch
>>> >>>>>>>>  >> provided wasn't sufficient... There were also user comments
>>> >>>> there
>>> >>>>>>>> about
>>> >>>>>>>>  >> it affecting non-facelets, or being fixed in the trunk -
>>> both
>>> >>>>>>>> statements
>>> >>>>>>>>  >> which are definately not true for my issue.
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> How involved do you think the fix for this would be? Could
>>> >>>> it be
>>> >>>>>>>> coded
>>> >>>>>>>>  >> in a couple of hours? Should I attempt to write a patch
>>> to fix
>>> >>>>>> this?
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> [1] http://issues.apache.org/jira/browse/TOMAHAWK-523
>>> >>>>>>>>  >>
>>> >>>>>>>>  >> Mike Kienenberger wrote:
>>> >>>>>>>>  >> > I think there are already bug reports open on this for
>>> >>>> Tomahawk,
>>> >>>>>>>> but
>>> >>>>>>>>  >> > you should make sure that this is the case, opening
>>> one if
>>> >>>>>>>> necessary.
>>> >>>>>>>>  >> >
>>> >>>>>>>>  >> > My guess it that the jsp tag handler for t:dataTable
>>> is not
>>> >>>>>> using
>>> >>>>>>>>  >> > standard pass-through code to initialize the
>>> rowStyleClass
>>> >>>>>>>> attribute
>>> >>>>>>>>  >> > on the t:dataTable component.
>>> >>>>>>>>  >> >
>>> >>>>>>>>  >> > The fix would be to rewrite the component and tag
>>> handler so
>>> >>>>>>>> that the
>>> >>>>>>>>  >> > tag handler isn't doing anything beyond passing the
>>> >>>> arguments
>>> >>>>>>>> through
>>> >>>>>>>>  >> > unchanged.
>>> >>>>>>>>  >> >
>>> >>>>>>>>  >> > On 2/15/07, Jeff Bischoff <[EMAIL PROTECTED]> wrote:
>>> >>>>>>>>  >> >> Greetings,
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> There is a CSS trick with the Tomahawk extended
>>> >>>> dataTable that
>>> >>>>>>>> allows
>>> >>>>>>>>  >> >> the selected row to be highlighted (or some similar
>>> >>>> things). It
>>> >>>>>>>> works
>>> >>>>>>>>  >> >> great in JSP, and has been passed around on the myfaces
>>> >>>> mailing
>>> >>>>>>>>  >> list and
>>> >>>>>>>>  >> >> wiki for some time. The trick goes something like this:
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> <t:dataTable id="TheDataTable"
>>> >>>>>>>>  >> >>     ...
>>> >>>>>>>>  >> >>     rowClasses="oddRow,evenRow"
>>> >>>>>>>>  >> >>
>>> rowStyleClass="#{dataTableBacking.selectedRowIndex ==
>>> >>>>>>>> rowIndex ?
>>> >>>>>>>>  >> >> 'highlightRow' : null}"
>>> >>>>>>>>  >> >>     rowIndexVar="rowIndex"
>>> >>>>>>>>  >> >>     .../>
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> Unfortunately, when I recently converted my application
>>> >>>> from
>>> >>>>>>>> JSP to
>>> >>>>>>>>  >> >> Facelets, this trick no longer works. (Fortunately, this
>>> >>>> is one
>>> >>>>>>>> of the
>>> >>>>>>>>  >> >> only things that stopped working!) I have heard from
>>> other
>>> >>>>>>>> users on
>>> >>>>>>>>  >> the
>>> >>>>>>>>  >> >> myfaces mailing list who also can't get this to work
>>> under
>>> >>>>>>>> facelets.
>>> >>>>>>>>  >> >> Apparently, the "rowIndex" variable that t:dataTable
>>> >>>> creates
>>> >>>>>>>> can't be
>>> >>>>>>>>  >> >> resolved in the rowStyleClass expression, even though it
>>> >>>>>> works for
>>> >>>>>>>>  >> >> components who are children of the table.
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> Any idea why this would be different under Facelets?
>>> I am
>>> >>>>>>>> thinking of
>>> >>>>>>>>  >> >> opening a JIRA issue on myfaces project, since this is
>>> >>>> their
>>> >>>>>>>> custom
>>> >>>>>>>>  >> >> component, but wanted to bounce for ideas here first.
>>> Any
>>> >>>>>>>> suggested
>>> >>>>>>>>  >> >> workarounds?
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> Regards,
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >> Jeff Bischoff
>>> >>>>>>>>  >> >> Kenneth L Kurz & Associates, Inc.
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >>
>>> >>>>>>>>  >> >>
>>> >>>>>>>>
>>> >>>>
>>> ---------------------------------------------------------------------
>>> >>>>>>>>  >> >> 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]
>>> >>>>>>>>  >
>>> >>>>>>>>  >
>>> >>>>>>>>  >
>>> >>>>>>>>  >
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>>
>>> >>>>>>
>>> >>>>>
>>> >>>>>
>>> >>>>
>>> >>>>
>>> >>>
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >
>>> >
>>> >
>>> >
>>>
>>>
>>>
>>
>>
>>
>
>
>
>
>



Reply via email to