search

angular-ui / ui-ace

This directive allows you to add ACE editor elements.
http://angular-ui.github.io/ui-ace
MIT License
578 stars 172 forks source link

ui-ace editor options set after, rather than before, onLoad callback function is called #93

Closed ispringer closed 9 years ago

ispringer commented 9 years ago

In our app, we have a page with 3 ui-ace editors on the same page - one in html mode, one in css mode, and one in javascript mode. We use the same onLoad callback function for all three, since we want to set most of the same ACE options. But we also want to do a couple things specific to which editor is being configured. In order to determine, in the onLoad function, which of the three editors was being configured, we checked if the editor's mode was html, css, or javascript.

We recently upgraded Angular from 1.2 to 1.3, and ui-ace from 1.1.2 to 1.2.1. After these upgrades, the code described above stopped working, because the onLoad function was being called before, rather than after, the ui-ace options specified in the html had been applied.

Run the below plunk with the browser console open and watch the logging. Notice, that in onchange, the mode specified via ui-ace options in the html has not yet been set on the ACE editor (it's still set to the default mode, "text"), but after a $timeout, the mode has been set to "css".

http://plnkr.co/edit/QGr1j8jjRCVsaTVDveEA?p=preview

The question is, does it make more sense to call the onLoad callback before or after setting the ui-ace options on the ACE editor. I'm thinking setting them before might be more intuitive. What do you think?

Whatever is decided, I think it would be good to document the order in the README.

douglasduteil commented 9 years ago

Wow I didn't see that one. Like you I clearly thing that it's better to have the onLoad callback called at the end of the options process.

douglasduteil commented 9 years ago

Whatever is decided, I think it would be good to document the order in the README.

Give me your feedback on https://github.com/angular-ui/ui-ace/pull/94. And make a PR with the doc update if you have the time too :+1: