Potential clash between ChoiceRenderer and IJQueryTemplate in newDataSource

[reckart]

One can easily provoke a conflict between a ChoiceRenderer and a template being set on a ComboBox (Kendo).

Let's say we have a domain object with a property "name".

We declare a ComboBox roughly as

box = new ComboBox("box", model, choices, new ChoiceRenderer("name")) {
  protected IJQueryTemplate newTemplate() {
    return new IJQueryTemplate()  {
      // snip
      public List<String> getTextProperties() {
        return Arrays.asList("name");
      }
    };
  }
});

This will cause the ComboBox.newDataSource() method to create a JSON string like:

{ name: "X", ds_value: "Y", name: "X" }

which will trigger an error in the browser:

Attempted to redefine property 'name'

It would be nice if such a thing could be cought already on the server side. E.g. ComboBox could check if a field returned by getTextProperties() is the same as renderer.getTextField() / rendered.getValueField() and skip it as not to duplicate it in the JSON string.

[sebfz1]

Hi Richard,

Good catch, thanks! I fixed it, that's the good news...

The bad news is that I already did a path-release this morning (6.20.1), so I cannot perform another patch-release today...

So you have 2 options:

• Use 6.20.2-SNAPSHOT, currently deploying
• Do not override #getTextProperties returning name if already part of the ChoiceRenderer (workaround)

Thanks & best regards, Sebastien.

[Patrick1701]

Hi Sebastien, since this change, it seems the method

TextRenderer
    @Override
    public String getText(T object, String expression)

is not called anymore in AutoCompleteTextFields choice rendering.

What was possible since then, is to implement a custom TextRenderer for AutoCompleteTextFields having a custom getText()-method to do something like this:

    @Override
    protected IJQueryTemplate newTemplate()
    {
        return new IJQueryTemplate() {

            private static final long serialVersionUID = 1L;

            /**
             * The template text will be enclosed in a <script type="text/x-jquery-tmpl" />.
             * You can use the "\n" character to properly format the template.
             */
            @Override
            public String getText()
            {
                return  "<div style=\"width:400px;\">${personAddressStreet}</div>";
            }

            @Override
            public List<String> getTextProperties()
            {
                return Arrays.asList("personAddressStreet");
            }

        };
    }

private static class PersonStreetSupportingRenderer extends TextRenderer<Person>{
        private static final long serialVersionUID = 1L;

        @Override
        public String getText(Person p, String expression) {

            if(expression.equals("personAddressStreet")){
                return p.getAddress().getStreet();
            }

            return super.getText(p, expression);
        }

    }

Sometimes I do this, because JQueryTemplate-TextProperty do not feature domain-graph-navigation, like ${person.address.street}

Should I open a bug issue for that? Or is my way overriding getText() to hacky to achieve such a behavior?

kind regards Patrick

[sebfz1]

Hi Patrick,

I didn't realized expressions like ${person.address.street} did not work in templates. I remember we had this problem on datatable's columns, I will try to find a solution for this one... (an issue will certainly helps ;))

I don't have any problem with the way your are using the TextRenderer, it actually looks like a regression from me (even I don't know exactly how/when I introduced this... and it seems to be previous to https://github.com/sebfz1/wicket-jquery-ui/commit/44b549995b81dbc6288cad01bc17e081fc96ff1b ...)

Could you please test 6.20.4-SNAPSHOT and open a different issue so I can commit properly? By chance, the fix will be in 6.21.0! ;)

Thanks in advance, Sebastien.

[Patrick1701]

Hi Sebastien, sorry for replying late... I will test 6.20.4-SNAPSHOT today afternoon.

best regards Patrick

[Patrick1701]

Hi, tested my custom way of handling those complex expressions using a renderer with 6.20.4-SNAPSHOT and it looks good to me. :) Thanx! What was the problem?

---

I didn't realized expressions like ${person.address.street} did not work in templates. I remember >we had this problem on datatable's columns, I will try to find a solution for this one... (an issue will >certainly helps ;))

Yes, I also remember the issue with the datatables. I'm not sure, if ${person.address.street} on js-side is a restriction/problem. Possibly JQuery-Templates already support that, if the json representation of AutoCompleteChoiceModelBehavior matches the expression. Not sure about this.

Do you think this could be an easy feature? Or would be an huge change in AutoCompleteChoiceModelBehavior, because somehow you have to response a kind of complex-object json, right?

kind regards Patrick

[sebfz1]

Hi Patrick,

Glad to read it works again

What was the problem?

The problem is that TextRenderer is designed to be bound to an object property (ie: one and only one property), or renders the object's #toString if none is supplied. My mistake was to call renderer#getText(object, property) instead of the built-in PropertyResolver for the other template-specified properties. It did not make sense. Basically it was exactly the same result, just a question of proper design. Now, you raised a good point about the advantage - even if it's to workaround a problem - of the first solution, so I restored it.

Do you think this could be an easy feature?

I unfortunately don't know yet, I need to have a closer look... For the datatable's issue we had to escape/unescape these properties (. to $ and $ to .). Please open an issue so I cannot try to forgot ;)

Best regards, Sebastien

[Patrick1701]

Hi Sebastien,

Do not override #getTextProperties returning name if already part of the ChoiceRenderer

I'm not sure, if this works for TextRenderer as well...

I defined a TextRenderer and removed the particular property from my override of #getTextProperties, but the property of the TextRenderer is not available on the response json of my AutoCompleteTextField.

It seems for AutoCompleteTextFields this looks like a bug... but not sure. What do you mean? Should I open an new bug issue?

best regards Patrick

[Patrick1701]

Ah...

ChoiceModelBehavior contains this code also

    // ITextRenderer //
    builder.append(renderer.render(choice));

this is missing in AutoCompleteChoiceModelBehavior.

I think, thats it... do you confirm?

Thanx Patrick

[sebfz1]

Hi Patrick,

jQuery UI's AutoComplete is a special case where the value field-name should be named value, unlike the other cases in Kendo UI where the field-name is the supplied property-name (ie: myProperty). That's the reason why the property is not outputted to the JSON, because it is not needed (you can use the value json key).

That said, I understand this is misleading, so I will add builder.append(renderer.render(choice)); in the AutoCompleteChoiceModelBehavior...

Best regards, Sebastien.