Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Use $evalAsync instead of $apply to make sure ngModel gets updated once editor content changes #77

Closed
wants to merge 4 commits into from

Conversation

rebornix
Copy link
Contributor

@rebornix rebornix commented Dec 5, 2014

Currently we are using !scope.$$phase && !scope.$root.$$phase to avoid to call $apply if there is already a digest loop running. The ngModel binded to editor content would not get updated until users modify the content once again. If users try to access the ngModel binded to editor content, he would get an incorrect answer.

We can push our ngModel updating task into $evalAsync queue.It would be executed before rendering the DOM and it won't conflict with digest loop.

Attach angularjs event loop workflow.

workflow

Thanks.

@rebornix
Copy link
Contributor Author

rebornix commented Dec 5, 2014

Build failure was introduced before my change.

@SummerSun
Copy link

👍 👍

@formulahendry
Copy link

Make sense!

@PowerKiKi
Copy link
Contributor

ui-ace declare a dependency on Angular ~1.x, but I believe $evalAsync is not available in the 1.0 branch. So dependency should be updated accordingly in this PR.

@rebornix
Copy link
Contributor Author

rebornix commented Dec 8, 2014

@PowerKiKi I have updated angularjs dependency in bower.json and readme. The build is failing, I'll try to fix it.

@PowerKiKi
Copy link
Contributor

I am not sure what to think of that. Specifying a devDependency on "angular-mocks": "1.3.0" make bower resolves to angular to 1.3.0. So basically everything is on 1.3. Should we just set a dependency on angular 1.3 ? or keep this "in-between" situation ? I am not sure what is the best solution here...

@douglasduteil would you have an opinion on this ?

@rebornix
Copy link
Contributor Author

rebornix commented Dec 9, 2014

Finally I figure this out. From Angular-mocks 1.3.1 (and AngularJS 1.3.x), we have to provide the <div ui-ace> element with controller ngModel explicitly, otherwise it would not be loaded as we declare ng-model as optional in link phrase.

return {
      restrict: 'EA',
      require: '?ngModel',
      link: function (scope, elm, attrs, ngModel) {

@PowerKiKi I have just reverted bower.json back and our dependency is Angularjs >=1.1 from now on, not just 1.3.

@douglasduteil
Copy link
Contributor

Hi
That's right @PowerKiKi the bower dev dependencies is wrong.
@rebornix I'm surprise to see that much modification just for fixing the ngModel behavior I'll try to spend some time updating this repo in the week or the next.

@rebornix
Copy link
Contributor Author

rebornix commented Dec 9, 2014

@douglasduteil good to know you can have a look at this repo. I'll keep updating this pr if I find better ways to get rid of build failure and this bug.

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

Successfully merging this pull request may close these issues.

5 participants