Hi guys, I have a serious issue, and it should be fixed in my opinion. (not those type of cosmetic issues that you guys used to close to cleanup issues for material v1)
The issue is:
require
doesn't work in the directive when using md-virtual-repeat with an internal ng-repeat.
CodePen
Angular Versions:
Angular Version:
1.4.4 but tried with 1.5.8 and the issue is still thereAngular Material Version:
0.10.0 but 1.1.0-rc.5 doesn't work neitherAdditional Information:
Browser Type: *
ChromeBrowser Version: *
52.0.2743.116 (64-bit)OS: *
Mac OSJust a side comment: In my app I got this error message only for the first inner iteration of ng-repeat
. For the following iterations the required controller in my directive is there.
its smooth
Any progress on this? I've created it 8 days ago and not even an answer was given...
@iamisti - please always test with the latest release of Angular Material. Current == v1.1.0.
@crisbeto - here is an updated CodePen demonstrating the issue: http://codepen.io/team/AngularMaterial/pen/zBQNoZ?editors=1111
@iamisti this isn't how a directive's require
works. You can't require a normal controller, you have to require another directive's controller. Here's how your example breaks, even without using md-virtual-repeat
at all.
On the other hand, it seems like virtual repeat works fine when using require
properly (requiring another directive). Here's a fork of the original codepen, that shows it working.
Alternatively, you can also set the AppCtrl
as your directive's controller, if you're feeling adventurous. It still works.
@crisbeto Thanks for your answer, I'll check it out because I'm not sure it will solve my issue. Actually I am requiring another directive's controller in my real code and it brakes. Let me check it out and I'll come back if the issue is still there.
@iamisti I can reopen the issue if you post a codepen where it breaks with another directive's controller, but from my tests it seemed to be working fine.
@crisbeto @ThomasBurleson
Sorry for the delay. I could reproduce the issue with your changes too.
Look at the console, and see that the first row stopped processing due the error.
I see, I'll take another look at it.
Thank you, and also for the quick response!
Why removed from milestone? :( This is crucial for mdDataTable to work properly. Please fix this
The milestone was changed since 1.1.1 is already released. The new one is 1.1.2
is there any way to download an "unreleased version" but that does include this fix (before 1.1.2) ?
its really blocking development and ruining the mdDataTable.
@mikila85 this hasn't been fixed yet, it's only the issue milestone that was changed. I'll look at it when I have time.
After looking into it again, what is happening is that the virtual repeater takes the first child, removes it from the DOM and then uses it as a reference (see poolBlock_). This then causes the removed child to "miss" the parent when it looks for it's controller, causing it to throw the error (sidenote: I believe ngRepeat does something similar which doesn't really help in this situation). Changing this behaviour is pretty risky and can end up breaking a lot of components, which is why I'm hesitant with messing around with it.
That being said, there are a couple of workarounds that you can do, depending on your situation:
require: '^^?customDirectiveParent'
) and skip over any elements that can't find it(in this case it should only be the first one). This ends up breaking the initial render, however you can trigger it manually by getting the virtual repeat controller and calling updateSize
. Here's a forked codepen showing it action. Note that this was added to the parent directive's controller:var virtualRepeatResizeListener = $scope.$on('$md-resize-enable', function() {
$element.find('md-virtual-repeat-container').controller('mdVirtualRepeatContainer').updateSize();
virtualRepeatResizeListener();
});
Let me know if you need any help with these. I know they aren't ideal, but they're less risky than trying to re-engineer the entire component.
@crisbeto Thank you for your investigation and for the suggested solutions.
Unfortunately none of them are potentional fixes.
The 1st solution is not an option, since we are talking about table columns, e.g.: <td>
elements which can not be replaced with a simple div
or anything else.
The 2nd solution is only working because you made the parent scope optional and not because of the updateSize
workaround call. Still not fixing the issue. I have references to the parent controller (methods I want to use, states I need to get). Obviously making it optional doesn't help getting the value from the parent controller.
Do you have any other solution or suggestion?
Regarding the second solution, technically the parent controller should only be missing on the first element. Everything else, including a clone of the first one, should have the reference. Of course, you'd lose the validation from Angular, but it should work.
I know that the directive got executed, but if the parent element is unreachable, then it's basically lost data if I skip that row. I need the data from the parent controller.
The first row will still get rendered (check the codepen, it starts from 0). That being said, I'm not quite sure why methods from the customDirective
scope don't get executed in the template.
Thank you very much for your time and quick answers.
But again, the problem is that the parent controller is important during the execution of the child link function (yes, even for the first row). I'm kind of dependent of that, there are values I have to use, and I'm pretty sure I won't be the only one who will need it for the longer run. :)
Please angular material guys please fix this we need this to work and it is blocking our company from using the awesome mdDataTable.
Any news on this?
Not really. I haven't looked at it since the last time. As I mentioned a little bit above, this is tricky to solve without completely reworking the component and potentially introducing breaking changes.
I see, but it's already a big issue, isn't it?
Are you saying it will not be fixed, or are you saying it will be but with a bit of a delay/thinking more how to solve it?
I'm open to suggestions on how we could fix it, however I don't have a decent idea yet.
@iamisti I think I have a workaround for it, but I'm not sure whether it won't break other functionality. Since your project seems to be using the virtual repeater heavily, would you be able to test a custom build out to see if it helps, as well as whether anything else looks out of place?
Here's the build with the workaround.
Note that this ended up breaking some of our own unit tests, but that might be down to triggering a digest at the proper time. I'd like to see if this helps first, before spending the time for cleaning it up.
Hey @crisbeto, thank you for your help, tomorrow I'll try it out (on Monday), and will get back to you with the results!
Any luck with that custom build @iamisti?
@crisbeto I realize your build is from a few months old now, but your custom build fixed the same issue occurring in my application. Is there a workaround that would allow me to get it working without running a custom version?
The idea was to try this build out and put in a proper fix to Material @johnhok, however I never got any feedback on it. I'm really hesitant with touching the virtual repeater since it can break in all sorts of unexpected ways.
@crisbeto Now that you have feedback (from me), does that make you more comfortable with merging the change into a release?
I suppose. What kind of error were you getting. The same one as the ones in the examples above?
@crisbeto Exactly the same error. https://docs.angularjs.org/error/$compile/ctreq
The first element rendered in the list threw the error. If I set the require to optional, the first item doesn't get the controller, but all subsequent items do. I changed nothing within my code, and replacing Bower version of Angular Material with the version in your zipfile above suddenly fixed the problem.
If it means fixing the problem, I may be able to get my QA team to make sure nothing else is broken by the version change. Is that appealing at all?
As for a workaround, I've been able to find one that's feasible in my case. If I use element.parent().controller('directiveName')
instead of require
, and only call that on the next tick (after DOM has rendered, using $timout
), I can at that point access the controller, even from the first list item.
That's sort of what the fix does, if I remember correctly (it's been a couple of months). I'll have to do a diff, but I think I threw in a timeout somewhere to delay the initialization.
@crisbeto Okay. Just let me know what I can do to help. I'd be keen to remove the workaround from my code so it _legitimately_ works :smile:
This seems similar to https://github.com/angular/material/issues/7275 which has a good answer on StackOverflow (linked in this comment: https://github.com/angular/material/issues/7275#issuecomment-421088825).
I'm going to close this issue as we don't plan to make any risky changes like this in AngularJS Material and the project that reported this (https://github.com/iamisti/mdDataTable) hasn't seen any activity since Nov 2016.
It also looks like @jonahbron has a workaround for his case. If this is still a problem for you, please let me know.
Most helpful comment
@iamisti I think I have a workaround for it, but I'm not sure whether it won't break other functionality. Since your project seems to be using the virtual repeater heavily, would you be able to test a custom build out to see if it helps, as well as whether anything else looks out of place?
Here's the build with the workaround.
Note that this ended up breaking some of our own unit tests, but that might be down to triggering a digest at the proper time. I'd like to see if this helps first, before spending the time for cleaning it up.