myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Robinson (JIRA)" <...@myfaces.apache.org>
Subject [jira] [Reopened] (TRINIDAD-1940) Problems with the tr:forEach
Date Tue, 22 Jan 2013 17:20:12 GMT

     [ https://issues.apache.org/jira/browse/TRINIDAD-1940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Andrew Robinson reopened TRINIDAD-1940:
---------------------------------------


Found issues with the current code. Re-opening so that I can address them.
                
> Problems with the tr:forEach
> ----------------------------
>
>                 Key: TRINIDAD-1940
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1940
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>          Components: Components
>    Affects Versions: 2.0.0-beta-1
>            Reporter: Andrew Robinson
>            Assignee: Andrew Robinson
>             Fix For: 2.1.0-core
>
>
> The tr:forEach tag has issues when trying to use component references and trying to keep
the component state in sync with the items in a list. It also does not support Maps like the
c:forEach tag does.
> I seek to improve the forEach tag in Trinidad, and propose that JSF/JSTL does something
similar so that user's can really use the for each tag without issues. Some of the for each
problems are described in my blog:
> http://drewdev.blogspot.com/2008/08/cforeach-with-jsf-could-ruin-your-day.html
> I propose to address each of the issues with the JSP tag.
> First, reliable component references:
> <tr:forEach var="item" items="#{myBean.items}">
>   <tr:panelGroupLayout id="pgl1" layout="horizontal">
>     <tr:inputText id="it1" label="Enter value:" value="#{item.text}" autoSubmit="true"
/>
>     <tr:outputText id="ot1" value="Value is: #{item.text}" partialTriggers="it1" />
>   </tr:panelGroupLayout>
> </tr:forEach>
> The problem is that the author intended the output text component to partially updated
by the sibling input text when it changed value, but that is not the result. Instead, the
partial triggers is always "it1", but the generated input text components are "ot1", "ot1j_id_1"
and "ot1j_id_2". The result is that the output text components all partially update only when
the first input text is changed.
> Another problem is that the indexed value expressions and the var status map that is
put into the variable mapper by the for each loop is static.  Take the following example:
> <tr:panelGroupLayout id="pgl1" layout="scroll">
>   <tr:forEach var="item" items="#{myBean.items}" varStatus="vs">
>     <tr:outputText id="ot1" value="This is the last item in the list: #{vs.last}.
Item #{item.text}." />
>   </tr:forEach>
> </tr:panelGroupLayout>
> Say during the first pass, the items in the list are A, B and C and the page would look
like this:
> This is the last item in the list: false. Item A.
> This is the last item in the list: false. Item B.
> This is the last item in the list: true. Item C.
> Now, consider what the output would be if someone added an object D to the end of the
list during invoke application:
> This is the last item in the list: false. Item A.
> This is the last item in the list: false. Item B.
> This is the last item in the list: true. Item C.
> This is the last item in the list: true. Item D.
> Notice that both C and D think they are the last. The reason is that the UIComponentClassicTagBase
will find the components generated for A, B and C during the findComponent call. It will only
create a new component for D. When D is created, it will pick up the new variable status map
created by the for each tag in its value expressions. Because when three earlier components
were build, C was the last item, and the variable status map in their value expressions did
not reflect any changes. D gets the correct values since it was just created.
> -----------------
> I propose to reduce any work required by page developers to implement the for each loop
with re-ordering support (avoid having to use immediate EL in the ID attribute) and fix the
issues above.
> The proposal is that Trinidad Tag based components (those components created by UIXComponentELTag)
will be able to have key-based IDs automatically generated as a result of being in a for each
tag. So consider this example:
> <tr:forEach var="item" items="#{bean.items}" varStatus="vs">
>   <tr:inputText id="it1" value="#{item.value}" partialSubmit="true" />
>   <tr:outputText id="ot1" value="Value is: #{item.value}"
>     partialTriggers="it1_${vs.key}" />
> </tr:forEach>
> In this code, the IDs of the components would automatically pick up the item key. The
item key would be stored on the var status. For List this key would simply be the index, for
Map it would be the map key and CollectionModel would use the row key. UIXComponentELTag could
check to see if a parent tag desires to alter the component IDs, which in this case, the for
each would. With that set, the tags would alter the component IDs to append the key. For example,
"_" + key would be appended to each component ID (it1 would become it1_A if the key were "A").
> The for each tag would set the key into the variable mapper so that, instead of indexes,
the key would be used to evaluate the EL with the limitation that the key would be the index
for List, in which the current behavior would be retained of the component state staying with
the index rather than with the object.
> Due to the fact that the JSP does not perform any component reordering, the org.apache.myfaces.trinidad.change.ReorderChildrenComponentChange
component change class can be use to alter the component order and also notify the component
change manager of the reordering. This would require users that wish to reorder the items
in the list to ensure to both re-order the list as well as reorder the components by applying
a component change.
> Changes to UIXComponentELTag
> Due to the fact that UIComponentClassicTagBase is a JSF class that we cannot modify,
in order to fix the tr:forEach bugs, we need to work with what we can change. We will want
to follow this up with proposed changes to the JSF and potentially the JSTL specifications
to be able to make this type of functionality standard.
> Since UIXComponentELTag extends UIComponentClassicTagBase, it can call protected methods
to alter the ID of the tag. With the altering of the tag ID, the UIComponentClassicTagBase
will produce different component IDs. In order to pull this off, there needs to be a defined
contract between the for each tag and the Trinidad component tags. This contract could be
done with static methods on UIXComponentELTag:
> Proposed methods to be added to UIXComponentELTag
> /**
>  * Push a component ID suffix onto the page context to append to component IDs generated
by {@link UIXComponentELTag}.
>  * This will append the suffix on all components up to a and including the text naming
container.
>  * @param pageContext the JSP page context
>  * @param suffix the suffix to append to component IDs
>  */
> public static void pushComponentIdSuffix(PageContext pageContext, String suffix)
> {
>   ...
> }
> /**
>  * Pop the last component ID suffix added to the page context.
>  * @see #pushComponentIdSuffix(PageContext, String)
>  * @param pageContext the JSP page context
>  */
> public static void popComponentIdSuffix(PageContext pageContext)
> {
>   ...
> }
> The ForEachTag would then call these methods before and after the body processing respectively.
The idea would be that if a UIXComponentELTag created a naming container, it would clear any
suffixes for children of the naming container. This is because the IDs would already be unique
in the naming container, and there would be no reason to modify them, and it would also stop
these changes from affecting any IDs inside of included pages. The push/pop method would be
used to support a stack usage. This way, if nested for each tags are present, multiple suffixes
would be added to ensure that the IDs in one for each tag would conflict with another's.
> As this would be a documented behavior, users could leverage this API to ensure that
their component references (like the partialTriggers attribute) point to the correct component.
> Changes to ForEachTag
> The for each tag, with this proposal, would need to be modified to call the new methods
of UIXComponentELTag in doStartTag and doAfterBody functions.
> The tag will need to change to provide dynamic value expressions for the var status and
the var so that changes to the items will not break EL. This would involve using the key as
a reference instead of an index for non list based items. The var status would need point
to a component tree based attribute that could be updated in each request. For example, this
would allow new items to be added, and the last attribute to correctly reflect that new state.
> The for each tag would be change to support Map and CollectionModel as well, and adding
the key as a valid attribute on the varStatus object.
> Optionally skip this for List and array
> If we did not implement the ID suffixing for List (and arrays), existing applications
would not be affected. This is one consideration to have to ensure that backwards compatibility
is maintained. Another alternative is to use a web.xml configuration parameter and only introduce
the ID suffixing when the web XML parameter has enabled it for lists and arrays.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message