fix($compile): use correct parent element when requiring on html element #16647
Conversation
gkalpak
left a comment
gkalpak
left a comment
There was a problem hiding this comment.
You might be able to unit-test this by mocking .parent() to return {nodeType: NODE_TYPE_DOCUMENT} or something, but it would already be messy enough.
I think it's fine to only have the e2e test.
| @@ -0,0 +1,7 @@ | |||
| <!DOCTYPE html> | |||
| <html ng-app="test" require-directive parent-directive> | |||
There was a problem hiding this comment.
parentDirective isn't a parent directive. I would name it siblingDirective or notParentDirective to make it obvious.
(I was kind of confused at first.)
There was a problem hiding this comment.
Well it's supposed to be the parent Directive. The require directive requires it on one of its parent elements but also has it itself for the sake of the test.
There was a problem hiding this comment.
No, it's a bug in requireDirective 😛
There was a problem hiding this comment.
No, the user uses it incorrectrly! :D
Narretz
force-pushed
the
fix-ng-model-options-html
branch
from
396071a to
dd00eaf
Compare
|
This is taking longer than anticipated. Turns out you cannot access the FF logs on Selenium. I changed it to check for a DOM element instead of the error message. |
Narretz
force-pushed
the
fix-ng-model-options-html
branch
from
6eb77e8 to
874cd3d
Compare
| return { | ||
| require: '^^requireTargetDirective', | ||
| link: function(scope, element, attrs, ctrl) { | ||
| window.document.querySelector('#container').append(window.document.createTextNode(ctrl.content)); |
There was a problem hiding this comment.
You can also do: .textContent = ctrl.content 😁
| 'use strict'; | ||
|
|
||
| angular. | ||
| module('test', []). |
There was a problem hiding this comment.
I think it would be better if you overwrote $exceptionHandler to put the error message into a DOM element.
Then you could easily read the error in the test (and verify that it is what you expect).
There was a problem hiding this comment.
Good idea, didn't think of that
Narretz
force-pushed
the
fix-ng-model-options-html
branch
from
0256df7 to
0059b66
Compare
ghost
mentioned this pull request
3 participants
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?
Please check if the PR fulfills these requirements
Other information: