test(directives) Added more tests for contenteditable directive. And c... #436
Conversation
… commented one issue found
|
Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement). CLA is important for us to be able to avoid legal troubles down the road. For individuals (a simple click-through form): http://code.google.com/legal/individual-cla-v1.0.html |
|
@giovannicandido I still don't see a CLA. It is possible you signed it, but the email addresses don't match. Could you reply to this PR when you've signed? |
|
Achievement unlocked: CLA signature found! |
|
@jbdeboer I sign Again with the email giovanni@atende.info that is the email with the commits |
| @@ -308,4 +308,20 @@ class ContentEditableDirective extends InputTextLikeDirective { | |||
| // The implementation is identical to InputTextLikeDirective but use innerHtml instead of value | |||
| get typedValue => (inputElement as dynamic).innerHtml; | |||
| set typedValue(String value) => (inputElement as dynamic).innerHtml = (value == null) ? '' : value; | |||
|
|
|||
| @override | |||
| processValue() { | |||
There was a problem hiding this comment.
Could you explain why this method needs to be overridden? It seems to be identical to the method being overridden except it is not doing validation.
There was a problem hiding this comment.
To this pull request be merge, there is 3 possibilities:
1 remove this peace of code, because as you said it does nothing and let one test fail.
2 override this code to not update content that is not editable, as in the comment isContentEditable() do not work as expected in my tests
3 Remove this code and remove the "should not update test" ignoring this error. The browser don't enable fields for edit, on this case, only direct code can do this.
I leave the code to use the pull request for discussions.
|
Can not merged as tests are failing. Could you get them to pass and let us know. |
ghost
assigned
|
Punting on this PR for 0.9.5 -> moved to 0.9.6 |
… behavior of code is as expected
| _.compile("<p contenteditable probe='p'></p>"); | ||
| _.rootScope.$digest(); | ||
| Element element = _.rootScope.p.element; | ||
| expect(element.isContentEditable).toBeTruthy(); |
There was a problem hiding this comment.
@mhevery This test fails but shouldn't. This is the source of all problems. Do you think this could be _.compile bug or should I initialise something in the test?
There was a problem hiding this comment.
I am not sure. I don't have time to further look at this, and it can not be merged in the current state.
If yo think it is a bug can you work around it somehow in the test or file an issue with http://dartlang.org and add link here for reference.
There was a problem hiding this comment.
I will dive into it, to locate the origin of problem and submit a bug or solution for the dart team, or the karma team, or find a workaround.
There was a problem hiding this comment.
awesome!
On Thu, Jan 30, 2014 at 1:51 PM, Giovanni Candido da Silva <
notifications@github.com> wrote:
In test/directive/ng_model_spec.dart:
_.rootScope.$digest();
_.rootScope.$apply('model = "Content"');
expect((_.rootElement as dom.DivElement).query("p").innerHtml).toEqual('');
- }));
- /**
\* If this test don't pass, several contentEditable test will fail*/- it("should the context contenteditable be truthy", inject((){
_.compile("<p contenteditable probe='p'></p>");_.rootScope.$digest();Element element = _.rootScope.p.element;expect(element.isContentEditable).toBeTruthy();I will dive into it, to locate the origin of problem and submit a bug or
solution for the dart team, or the karma team, or find a workaround.—
Reply to this email directly or view it on GitHubhttps://github.com//pull/436/files#r9324997
.
|
Ping, any update on this? |
|
Separated tests for the problem: https://github.com/giovannicandido/darthtml-content-editable-test |
|
Moving to 0.9.8 |
|
Any update on this? |
|
Closing due to inactivity |
Hi,
After some time beating my head trying to make all testes pass. I find that element.isContentEditable do not work.
If someone have some Idea for make the "should NOT update paragraph on html edited" test pass, or maybe is better ignore this.