ICEfaces
  1. ICEfaces
  2. ICE-2031

Selected options converter improvement for MenuRenderer

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.7DR#3, 1.7
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      All

      Description

      I'm proposing a little improvement for the MenuRenderer of ICEFaces, which it's
      very important for our application.
      We need using complex objects associated to a UISelectItem.
      Usually, the value of an UISelectItem is a simple object (String, Integer,etc);
      using a complex object is possible if there is a converter associated to the
      UISelectOne component, which generates a "key" which identifies the complex
      object in the getAsString() method of the Converter. In fact, in the generated
      HTML page, the complex object has to be represented as a simple string in the
      VALUE attribute of OPTION element.
      But there's a problem during the rendering of the UISelectOne component, in fact
      no option is selected, even if the model of the managed-bean is set to a value
      which should match one of values contained in the list of the UISelectItems
      component.
      This happens when the current model and the matching element contained by the
      UISelectItems component have the same "key", but they are two different
      instances. The "key" is the string returned by the getAsString() method of the
      associated Converter.
      The class com.icesoft.faces.renderkit.dom_html_basic.MenuRenderer is responsible
      of writing the SELECTED attribute for the OPTION element which corresponds to
      the model actually selected.
       
      Looking at the
      com.icesoft.faces.renderkit.dom_html_basic.MenuRenderer.renderOption(FacesContext,
      UIComponent, SelectItem, Element) method, when the submittedValue is not null,
      the following code is executed:
       

      Object selectedValues = getCurrentSelectedValues(uiComponent);

       isSelected = isSelected(selectItem.getValue(), selectedValues);
       
      The logic which determines if an option is selected does not use converted
      values (it is not based on string comparison, but on generic Object comparison),
      but the value obtained by the UIComponent is used.
      I send you a patched version of the MenuRenderer class (version 1.5.3 of
      ICEFaces, but the situation is identical in 1.6.0), which always uses the
      converted value for calculating the selected item.
      In fact, the formatComponentValue() method is called not only for the
      submittedValue, but also for the value of SelectItem and UISelectOne: they are
      treated in the same way.
       
      If you don't like this solution and you prefer making comparison between the raw
      value of components, I ask you if in the next releases it's possible to isolate
      the code which contains the logic of determining the selected item, putting it
      in a protected method; so I can easily change the logic in a sub-class of the
      MenuRenderer.
      For example, the
      com.icesoft.faces.renderkit.dom_html_basic.MenuRenderer.renderOption(FacesContext,
      UIComponent, SelectItem, Element) method can be modified as in the following
      code, adding the new isValueSelected method, which is protected, but your
      original logic is completely unchanged:
       
          protected void renderOption(FacesContext facesContext,
                                      UIComponent uiComponent,
                                      SelectItem selectItem, Element optionGroup)
                  throws IOException {
       
              DOMContext domContext =
                      DOMContext.attachDOMContext(facesContext, uiComponent);
       
              Element select = (Element) domContext.getRootNode();
              Element option = domContext.createElement("option");
       
              if (optionGroup == null) {
                  select.appendChild(option);
              } else {
                  optionGroup.appendChild(option);
              }
       
              String valueString = formatComponentValue(facesContext, uiComponent,
                                                        selectItem.getValue());
              option.setAttribute("value", valueString);
       
            /* Now the logic is here!!!! */
            boolean isSelected = isValueSelected(facesContext,selectItem,uiComponent);
       

              if (isSelected) {
                  option.setAttribute("selected", "selected");
              }
              if (selectItem.isDisabled()) {
                  option.setAttribute("disabled", "disabled");
              }
       
              Document doc = domContext.getDocument();
              Text labelNode = doc.createTextNode(selectItem.getLabel());
              option.appendChild(labelNode);
          }
       
          protected boolean isValueSelected(FacesContext facesContext, SelectItem
      selectItem,UIComponent uiComponent){
           boolean isSelected;
           
           String valueString = formatComponentValue(facesContext, uiComponent,
                      selectItem.getValue());
            Object submittedValues[] =
                   getSubmittedSelectedValues(uiComponent);
           
            if (submittedValues != null) {
                  isSelected = isSelected(valueString, submittedValues);
              } else {
                  Object selectedValues =
                          getCurrentSelectedValues(uiComponent);
                      isSelected = isSelected(selectItem.getValue(), selectedValues);
              }
           
           return isSelected;
          }
      1. Car.java
        0.7 kB
        yip.ng
      2. CarConverter.java
        1.0 kB
        yip.ng
      3. SelectionTagsBean.java
        12 kB
        yip.ng

        Activity

        Tyler Johnson created issue -
        Hide
        Alexander Merk added a comment -

        That's a very good explanation. I have the same problem. My Problem is the following:

        <ice:selectOneMenu value="$

        {myBean.prop123}

        ">
        <f:selectItem itemValue="123" itemLabel="foo" />
        </ice:selectOneMenu>

        the property "prop123" is from type "MyClass" and implements the "equal" method. Now the item will never be selected because you always convert the value of the selectItem to an String and execute the equals method of the string. In the case to create via java code the select items with value of type "MyClass" to force the icefaces implementation to call the equals method of the MyClass Object is also wrong, because the formatComponentValue convert it to an String.

        If I can override the selection code I will be happy or you change the code to executes the equals from the value binding object.

        Show
        Alexander Merk added a comment - That's a very good explanation. I have the same problem. My Problem is the following: <ice:selectOneMenu value="$ {myBean.prop123} "> <f:selectItem itemValue="123" itemLabel="foo" /> </ice:selectOneMenu> the property "prop123" is from type "MyClass" and implements the "equal" method. Now the item will never be selected because you always convert the value of the selectItem to an String and execute the equals method of the string. In the case to create via java code the select items with value of type "MyClass" to force the icefaces implementation to call the equals method of the MyClass Object is also wrong, because the formatComponentValue convert it to an String. If I can override the selection code I will be happy or you change the code to executes the equals from the value binding object.
        Ken Fyten made changes -
        Field Original Value New Value
        Fix Version/s 1.7DR#1 [ 10100 ]
        Assignee Mark Collette [ mark.collette ]
        Ken Fyten made changes -
        Fix Version/s 1.7 [ 10080 ]
        Fix Version/s 1.7DR#1 [ 10100 ]
        Philip Breau made changes -
        Philip Breau made changes -
        Ken Fyten made changes -
        Assignee Mark Collette [ mark.collette ] Yip Ng [ yip.ng ]
        Ken Fyten made changes -
        Fix Version/s 1.7DR#3 [ 10112 ]
        Fix Version/s 1.7 [ 10080 ]
        Assignee Priority P1
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #15237 Thu Nov 22 16:39:40 MST 2007 yip.ng ICE-2031
        Changed to allow more flexibility in the conversion and matching of values between the SelectOne and SelectMany components and their SelectItem's
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/faces/renderkit/dom_html_basic/SelectManyCheckboxListRenderer.java
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/faces/renderkit/dom_html_basic/RadioRenderer.java
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/faces/renderkit/dom_html_basic/MenuRenderer.java
        Hide
        yip.ng added a comment -

        Changes made based on the suggestions. Some more changes added to avoid breaking existing applications.

        With these changes, objects of the same type should match correctly with the proper equals() method.

        Even objects of different types should match correctly provided the toString() or Converter.getAsString() method return the same value. But this only works for rendering and submission. The glitch is that in the standard JSF implementation the submitted value is converted to the type of the component value and then validated against the selected items, using just the equals() method, like this:

        convertedSubmittedValue.equals(selectItem.getValue())

        So if the component value type is MyClass and the item value is the string "123", the condition (Myclass.equals("123")) is usually false, unless you are willing to deviate from the usual equals() method implementation to accommodate this:

        private String key;
        ...
        public boolean equals(Object o)

        { ... if (key.equals(o)) return true; ... }

        When this validation fails, you have to use the <h:messages/> tag to show the error message. Otherwise you would be left scratching your head wondering why everything seemed to work fine but the component binding value just didn't get updated.

        Therefore, we should use the same type for the component value and the select items.

        Show
        yip.ng added a comment - Changes made based on the suggestions. Some more changes added to avoid breaking existing applications. With these changes, objects of the same type should match correctly with the proper equals() method. Even objects of different types should match correctly provided the toString() or Converter.getAsString() method return the same value. But this only works for rendering and submission. The glitch is that in the standard JSF implementation the submitted value is converted to the type of the component value and then validated against the selected items, using just the equals() method, like this: convertedSubmittedValue.equals(selectItem.getValue()) So if the component value type is MyClass and the item value is the string "123", the condition (Myclass.equals("123")) is usually false, unless you are willing to deviate from the usual equals() method implementation to accommodate this: private String key; ... public boolean equals(Object o) { ... if (key.equals(o)) return true; ... } When this validation fails, you have to use the <h:messages/> tag to show the error message. Otherwise you would be left scratching your head wondering why everything seemed to work fine but the component binding value just didn't get updated. Therefore, we should use the same type for the component value and the select items.
        Hide
        yip.ng added a comment -

        Sample select item class, converter class and bean class used for testing in component showcase.

        Show
        yip.ng added a comment - Sample select item class, converter class and bean class used for testing in component showcase.
        yip.ng made changes -
        Attachment Car.java [ 10745 ]
        Attachment CarConverter.java [ 10746 ]
        Attachment SelectionTagsBean.java [ 10747 ]
        Hide
        yip.ng added a comment -

        How to test:

        First, test the selection components of the component showcase to make sure that existing applications still work as before.

        Then, test using objects instead of just strings in the selection components. See the attached Java files for an example. In this example, we use a car class and a car converter class instead of just strings to do the car selection. Just replace SelectionTagsBean.java and add Car.java and CarConverter.java in the component showcase application. (Look in the files for the package location.) You also need to make a change in component-showcase\web\inc\components\selectionTags.jspx to specify the converter:

        <ice:selectManyMenu ... converter="#

        {selectionTags.carConverter}

        " ... >

        Also, add a <h:messages/> anywhere to display any hidden conversion or validation errors in case something went wrong with the submitted value conversion and validation.

        Show
        yip.ng added a comment - How to test: First, test the selection components of the component showcase to make sure that existing applications still work as before. Then, test using objects instead of just strings in the selection components. See the attached Java files for an example. In this example, we use a car class and a car converter class instead of just strings to do the car selection. Just replace SelectionTagsBean.java and add Car.java and CarConverter.java in the component showcase application. (Look in the files for the package location.) You also need to make a change in component-showcase\web\inc\components\selectionTags.jspx to specify the converter: <ice:selectManyMenu ... converter="# {selectionTags.carConverter} " ... > Also, add a <h:messages/> anywhere to display any hidden conversion or validation errors in case something went wrong with the submitted value conversion and validation.
        yip.ng made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Ken Fyten made changes -
        Fix Version/s 1.7 [ 10080 ]
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Assignee Priority P1
        Assignee Yip Ng [ yip.ng ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Tyler Johnson
          • Votes:
            2 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: