aurelia / binding

A modern databinding library for JavaScript and HTML.
MIT License
111 stars 96 forks source link

Matcher ignored within repeater #764

Closed RomkeVdMeulen closed 4 years ago

RomkeVdMeulen commented 4 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: While loopig over objects with repeat.for and binding radio inputs with model.bind and checked.bind, the proper radio is selected if the selected object instance is a member of the looped collection. However, when using matcher.bind like in the docs the radio is not selected.

Strangely enough while researching this bug I found that if I manually reference an object by index, or hard-code an equivalent object in the model binding, the radio is selected, even within the repeater scope. See my reproduction: https://github.com/RomkeVdMeulen/aurelia-binding-matcher-bug

I also noticed that for the radios that refer to the repeater local in their model binding, their matcher property is undefined. All radios with a model binding that manually refers to some object instance has the proper value in their matcher property.

The matcher should be called for each iteration of the repeater and the radio corresponding to the matching object should be selected.

bigopon commented 4 years ago

@RomkeVdMeulen is this issue similar to yours https://github.com/aurelia/templating-resources/issues/388

If so, can you try the suggestion there?

RomkeVdMeulen commented 4 years ago

Let me just summarize my results:

export class App {
  constructor() {
    this.products = [
      {id: 0, name: "Motherboard"},
      {id: 1, name: "CPU"},
      {id: 2, name: "Memory"},
    ];

    this.selectedProduct = { id: 1, name: 'CPU' };

    this.productMatcher = (a, b) => {
      console.log("match", a, b);
      return a.id === b.id;
    }
  }
}
<template>
  <form>
    <h4>Products (without repeater)</h4>
    <label>
      <input type="radio" name="group1"
              model.bind="products[0]"
              matcher.bind="productMatcher"
              checked.bind="selectedProduct">
      Motherboard
    </label>
    <label>
      <input type="radio" name="group1"
              model.bind="products[1]"
              matcher.bind="productMatcher"
              checked.bind="selectedProduct">
      CPU
    </label>
    <label>
      <input type="radio" name="group1"
              model.bind="products[2]"
              matcher.bind="productMatcher"
              checked.bind="selectedProduct">
      Memory
    </label>

    <p>Selected product: ${selectedProduct.id} - ${selectedProduct.name}</p>

    <h4>Products (repeater)</h4>

    <template repeat.for="product of products">
      <label>
        <input type="radio" name="group2"
            model.bind="product"
            matcher.bind="productMatcher"
            checked.bind="selectedProduct">
        ${product.name} (ID ${product.id})
      </label>

      <template if.bind="$last">
        <p>
          Manually selecting the index:
          <input type="radio" name="group3"
                  model.bind="products[1]"
                  matcher.bind="productMatcher"
                  checked.bind="selectedProduct">
        </p>
        <p>
          Hardcoded model:
          <input type="radio" name="group4"
                  model.bind="{id: 1}"
                  matcher.bind="productMatcher"
                  checked.bind="selectedProduct">
        </p>
        <p>Matcher in this scope is: ${productMatcher}</p>
      </template>

  </form>
</template>

Result:

screenshot of the result of running this code

bigopon commented 4 years ago

@RomkeVdMeulen We posted the comment almost at the same time, so to make sure it's not missed,

@RomkeVdMeulen is this issue similar to yours aurelia/templating-resources#388

If so, can you try the suggestion there?

RomkeVdMeulen commented 4 years ago

@bigopon It is similar, and the suggestion works. I'll just close this as a duplicate.

That is a very obscure setting though. When was this introduced?

RomkeVdMeulen commented 4 years ago

Like @pkozul said in the referenced issue, I could not have found that setting if you hadn't pointed it out. Could a note be added in the docs somewhere? Otherwise I imagine more bug reports like this will keep coming in.

bigopon commented 4 years ago

It's added here https://github.com/aurelia/templating-resources/pull/379

Sorry for the doc, and it would be great if you could help with that

RomkeVdMeulen commented 4 years ago

Sorry for the late reaction. I'd love to help with doc on this, but I'm not familiar with the material. For one, I didn't know until I read your PR that it was even possible to add a matcher to a repeater.