Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firefox - if.binds not working in repeat.for elements #250

Closed
JoshHarris opened this issue Dec 1, 2015 · 19 comments
Closed

Firefox - if.binds not working in repeat.for elements #250

JoshHarris opened this issue Dec 1, 2015 · 19 comments

Comments

@JoshHarris
Copy link

Ran the below code in the navigation app with a simple list of strings to recreate. Basically the show.bind works, but if.bind does not. This is in firefox, but works fine in chrome.

if - list:
  <ul>
    <li repeat.for="item of someList"
      if.bind="item">
      <div>${item}</div>
    </li>
  </ul>
show - list:
  <ul>
    <li repeat.for="item of someList"
      show.bind="item">
      <div>${item}</div>
    </li>
  </ul>
@EisenbergEffect
Copy link
Contributor

You cannot place an if and a repeat on the same element. You need to nest them. You can nest them by using a template element. This is true of any attributes which are "template controllers" such as repeat, if and with.

@JoshHarris
Copy link
Author

@EisenbergEffect Why does it work in chrome then? Is that also true for show.bind and any other bindable or just special cases like if.bind?

if you cant use if/show.bind on the same element as the repeat.for how would one output a little of items and only display the ones that meet a specific criteria, without removing them from the list before hand?

@EisenbergEffect
Copy link
Contributor

I have no clue why it works in Chrome. It shouldn't. show is not a template controller, so it can be placed along side if or repeat. Template Controllers are literally attributes that transform the DOM that they are on into an html template and then control how that template is instantiated.

If you want to use if and repeat together, you must nest them. Here's your code with the correct usage:

<ul>
    <li repeat.for="item of someList">
      <template if.bind="item">
        <div>${item}</div>
      </template>
    </li>
  </ul>

Any template controller can be used directly on an HTML element or on a Template element.

@JoshHarris
Copy link
Author

Good to know, thanks for the info.

The code above would still have empty li elements if the if.bind was false. Suppose show.bind could be applied to the li and the template if.bind would control the execution of the component instead the li

Have a good one :)

@EisenbergEffect
Copy link
Contributor

You could use a value converter to filter your list as well. Then you would only have li's for truthy values.

@JoshHarris
Copy link
Author

True enough

@timbell
Copy link

timbell commented Jan 22, 2016

I just came across the same - all was working in Chrome and then I tested Firefox and IE. I've ended up doing the equivalent of:

<ul>
    <template repeat.for="item of someList">
        <li if.bind="item">
            <div>${item}</div>
        </li>
    </template>
</ul>

Doing the repeat.for on the template and the if.bind on the li avoids the empty li elements

@JoshMcCullough
Copy link

@EisenbergEffect Can you elaborate more on why mixing template bindings shouldn't work? We're currently mixing if / repeat.for, etc. and have not seen issues in IE, Firefox, or Chrome. Thanks!

@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Jun 30, 2016

There's a difference between whether you want to "if" a list in its entirely or "if" individual items in a list, which is why the order would matter in that case. However, I don't believe that all browsers preserve the ordering of the attributes. So, it could be that you want a particular order, but that the browser swaps those around at runtime.

@matthewma7
Copy link

I just experienced this issue myself, including the inconsistency between Chrome and firefox(IE 11).
Knockout will throw an error when you try to have multiple what it called "Control flow" bindings on the same element.
Is it possible to throw an error message without adding too much overhead to checking multiple control flow in Aurelia?

Love Durandal and now love Aurelia

@EisenbergEffect
Copy link
Contributor

@morioma Can you create a separate issue to let us track this? I believe we may have it handled through some new community tooling that "lints" templates, but I'd like to track it separately and make sure.

@pasky
Copy link

pasky commented Jul 6, 2016

Hmm, I'm confused about this, sorry - I have two cases where I'm combining repeat.for and if.bind, tr and option. If I use the nested template trick, I get empty elements, so that workaround doesn't seem to work?

I could of course do a lot of pre-processing in the javascript model, but that would imho mean a big deficiency in Aurelia as this would mean a lot of boilerplate code, instead of self-explaining combination of two attributes.

(Maybe if there's an easy way to use a value converter in template context, an example would be immensely helpful. :)

@pasky
Copy link

pasky commented Jul 6, 2016

Ok, to give a concrete example, my original line

<option repeat.for="cl2 of task.classes" model.bind="cl2.id" if.bind="shownclass.id != cl2.id">

got replaced by

<require from="./_filter"></require>
<option repeat.for="cl2 of task.classes | filterNE:'id':shownclass.id" model.bind="cl2.id">${cl2.name}</option>

with _filter.js being

export class FilterNEValueConverter {
  toView(value, attr, other) {
    return value.filter(e => e[attr] !== other);
  }
}

All if.binds containing inequalities, which are quite a common case I guess. I guess not as bad as I feared... :)

Still, I'd consider this as a bug - the code is rather more convoluted and less maintainable than if Aurelia had a builtin for what I'd say is a first-class functionality (in my pretty simple app, I already had to replace three occurences of this - noticed only after testing in firefox).

@bdefore
Copy link

bdefore commented Nov 30, 2017

The useRuleConflictingAttribute (on by default) in https://github.com/MeirionHughes/aurelia-template-lint provides protection against inadvertently doing this.

@SalkiDixit
Copy link

SalkiDixit commented Mar 18, 2020

@EisenbergEffect Hi, I am trying to use show.bind inside repeat.

<div repeat.for="file of files">
      <div class="file-type-icon">
            <div show.bind="${file.type} === 'image'">This is image</div>
            <div show.bind="${file.type} === 'video'">This is Video</div>
       </div>
 </div>

This is not working..
please assist.

@bigopon
Copy link
Member

bigopon commented Mar 18, 2020

@SalkiDixit Can you try with

<div repeat.for="file of files">
      <div class="file-type-icon">
            <div show.bind="file.type === 'image'">This is image</div>
            <div show.bind="file.type === 'video'">This is Video</div>
       </div>
 </div>

${file.type} === 'image' is incorrect JS expression syntax, that's probably why it didn't work for you

@SalkiDixit
Copy link

@bigopon Thanks..its working this way.

@SalkiDixit
Copy link

SalkiDixit commented Mar 21, 2020

@bigopon Hi,

<div repeat.for="file of files">
  <div class="file-type-icon">
        <div show.bind="file.type === 'image/jpeg' || 'image/png'">This is image</div>
        <div show.bind="file.type === 'video/mp4'">This is Video</div>
   </div>
 </div>

I am trying to execute this piece of code. when i try to show video files, as per type only 'This is Video' should be shown, but Both are visible 'This is image' & 'This is video'.
I tried with if.bind also, still not working as intended,
Please assist.

@bigopon
Copy link
Member

bigopon commented Mar 21, 2020

It should be like this

<div repeat.for="file of files">
  <div class="file-type-icon">
        <div show.bind="file.type === 'image/jpeg' || file.type === 'image/png'">This is image</div>
        <div show.bind="file.type === 'video/mp4'">This is Video</div>
   </div>
 </div>

You missed file.type === after ||

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants