Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Too many ng-if 's #8721

Closed
binarykitchen opened this issue Aug 22, 2014 · 15 comments
Closed

Too many ng-if 's #8721

binarykitchen opened this issue Aug 22, 2014 · 15 comments

Comments

@binarykitchen
Copy link

For my app I want to add lots of meta tags like that (in the jade template):

        meta(property="og:type",        content="video.other",           data-ng-if="videomail.sent")
        meta(property="og:title",       content="{{videomail.subject}}", data-ng-if="videomail.sent")
        meta(property="og:url",         content="{{videomail.url}}",     data-ng-if="videomail.sent")

... and 20 more!

All of them should be shown if only one flag is set, namely videomail.sent.

I think defining data-ng-if="videomail.sent" for each line is somewhat cumbersome.

I wish there is a block element for the meta tags in the header. Something like that

head
        head-group(data-ng-if="videomail.sent)
            meta(property="og:type",        content="video.other")
            meta(property="og:title",       content="{{videomail.subject}}")
            meta(property="og:url",         content="{{videomail.url}}")

But there is no such tag. But maybe you know a better workaround for that?

@caitp
Copy link
Contributor

caitp commented Aug 22, 2014

you should be able to use ng-if-start and ng-if-end to do this (but the hard part is that the parser might move those nodes into <body> if they're the wrong name)

@caitp
Copy link
Contributor

caitp commented Aug 22, 2014

@binarykitchen did this work for you? I think we can close this

@pkozlowski-opensource
Copy link
Member

@caitp @binarykitchen this seems to be working correctly, at least in Chrome and FFox, guess we can close this one: http://plnkr.co/edit/tatbBjoS2sJr8Rrq4pIs?p=preview

@caitp
Copy link
Contributor

caitp commented Aug 22, 2014

Yep, looks like it! @binarykitchen hope that helps

@caitp caitp closed this as completed Aug 22, 2014
@binarykitchen
Copy link
Author

Thanks folks, that's nice but my case is a bit more complicated. I have an video object here that can come with different combinations. Here an example for a JADE file with AngularJS together:

        meta(property="og:type",            content="video.other",              data-ng-if-start="showMoreMetaTags && videomail")
        meta(property="og:title",           content="{{videomail.subject}}",    data-ng-if="videomail.subject")
        meta(property="og:url",             content="{{videomail.url}}",        data-ng-if="videomail.url")
        meta(property="og:description",     content="{{videomail.body}}",       data-ng-if="videomail.body")
        meta(property="og:image",           content="{{videomail.poster}}",     data-ng-if="videomail.poster")
        meta(property="og:image:type",      content="image/jpeg",               data-ng-if="videomail.poster")
        meta(property="og:image:width",     content=settings.video.size.width,  data-ng-if="videomail.poster")
        meta(property="og:image:height",    content=settings.video.size.height, data-ng-if="videomail.poster")
        meta(property="og:video",           content="{{videomail.mp4}}",        data-ng-if="videomail.mp4")
        meta(property="og:video:type",      content="video/mp4",                data-ng-if="videomail.mp4")
        meta(property="og:video:width",     content=settings.video.size.width,  data-ng-if="videomail.mp4")
        meta(property="og:video:height",    content=settings.video.size.height, data-ng-if="videomail.mp4", data-ng-if-end)

See, there is a data-ng-if-end at the end. This doesn't work but would be great. Any ideas / suggestions?

@pkozlowski-opensource
Copy link
Member

@binarykitchen your example looks wired, you shouldn't have ng-if between ng-if-start / ng-if-end and for sure not ng-if and ng-if-end on the same node. In any case, try to put things in plunker if you need help, you can start with the one I've provided.

@binarykitchen
Copy link
Author

Not really. I was just showing a real world example. I have a JavaScript object called videomail which sometimes has an subject, an url, a body etc. Hence the additional ng-ifs you are seeing. This looks similar like

if (videomail) {
  if (videomail.subject) {

  }

  if (videomail.url) {

  }

  if (videomail.body) {

  }
}

which is better and easier to maintain than

if (videomail && videomail.subject) {

}

if (videoamail && videomail.url) {

}

if (videomail && videomail.body) {

}

hope this helps to understand what I am after?

@caitp
Copy link
Contributor

caitp commented Aug 25, 2014

@binarykitchen isn't that exactly this sort of construct? http://jsbin.com/wuxoquvogegu/1/

You can have nested "ifs", so that should work for you

@binarykitchen
Copy link
Author

That's close but the last one line

<p ng-if-end></p>

will cause an empty p tag I don't want.

That's why it would be great to allow something like this:

<p ng-if="object.bar" ng-if-end>Bar: {{object.bardata}}</p>

@caitp
Copy link
Contributor

caitp commented Aug 26, 2014

Yeah --- as mentioned in the issue I just opened, there's a way around this, but it's not currently implemented :( It wouldn't be too much of a refactoring to make that work, though, so will just have to see what Igor thinks about it.

You might end up out of luck for supporting that, though. Does the extra node really hurt? why not just write a directive which removes it from the DOM?

@binarykitchen
Copy link
Author

Thanks @caitp !

Well, I fancy clean HTML and do not want to leave unused tags behind. I think adding a directive to remove unnecessary DOM afterwards is only a temporary solution.

I can wait until the refactoring is done.

@caitp
Copy link
Contributor

caitp commented Aug 26, 2014

The point that I make is --- it's not clear that the refactoring will happen, because comment directives are a pretty rare use case and it doesn't really make sense to add another feature to them. But, since comments are basically just elements, it might not be unreasonable to support it. Will have to see what Igor thinks, but it's probably not a priority for 1.3 and is quite unlikely to make it into 1.2

@binarykitchen
Copy link
Author

Oh right, thanks for the clarification. All good. Will just watch this thread for now.

@caitp
Copy link
Contributor

caitp commented Aug 26, 2014

Yeah just talked to Igor about it, as expected I don't think that's going to get fixed, so you're stuck with adding a directive to remove the wrapper nodes, or else just giving them a display: none style.

@binarykitchen
Copy link
Author

:(

Alright, a directive then - how would you write such a directive?

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

No branches or pull requests

3 participants