Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add editor HTML element to plugin bindings #131

Closed
bampakoa opened this issue Jul 18, 2016 · 11 comments
Closed

Add editor HTML element to plugin bindings #131

bampakoa opened this issue Jul 18, 2016 · 11 comments

Comments

@bampakoa
Copy link
Contributor

@stevermeister what do you think about passing the HTML element that hosts the editor in the bindings property of ngWigPlugin? In that way, we could overcome difficulties that appear when using multiple editors in a template and simplify the way to find in which editor the plugin belongs to.

For example, in clear-styles plugin, we use the following to find the actual editor element:

var ngWigElement = e.target.parentElement.parentElement.parentElement.parentElement.parentElement;
var container = angular.element(ngWigElement.querySelector('#ng-wig-editable'));

which is really error prone and difficult to read and test. It could be changed to something like:

var container = angular.element(this.hostElement);

@stevermeister
Copy link
Owner

why do we need it inside plugin?
it was nice to bind ngModel, but why element itself?

@bampakoa
Copy link
Contributor Author

I think it will be helpful in case someone wants to interact directly with the actual HTML element.

Take for instance the clear-styles plugin that uses an inefficient way to find the host element of the editor. Or even in the case of #130 where @sajithk was having problems with the focus on the editor. If he was to create a plugin that sets focus, he should find a way to decide which is the owner editor.

We could pass the ID of the element if we do not want to have the whole editor object being transferred between the bindings.

@stevermeister
Copy link
Owner

yes, "focus" is a good example.

but " clear-styles" should work only with ngModel, no?

@bampakoa
Copy link
Contributor Author

I think that ngModel contains the text along with any styles (e.g. p, li, br). If it is like that, there is no easy way to get the text only. except from textContent property of the element.

@stevermeister
Copy link
Owner

in this case if you want to work with DOM properties, you can just create virtual element, put content there and do textContent.
still not convinced.
I just want to provide properties that you really need, but not that you might want to have in some rare cases.

Could you provide at least on more example please?

@bampakoa
Copy link
Contributor Author

bampakoa commented Jul 20, 2016

I gave a second thought on the issue and I think it is not worth to provide it as a binding, for the reasons you have already mentioned.

Could you please provide an example with the use of the virtual element that you describe? I would be really interested to integrate such a solution to the clear-styles plugin.

Alternatively, we could provide a service that gets the active editor and each plugin could inject it when needed.

@stevermeister
Copy link
Owner

let div = document.createElement('div')
div.innerHTML = htmlContent;
div.textContent

@bampakoa
Copy link
Contributor Author

I am struggling to understand how can this be used in clear-styles plugin. Could you please elaborate a bit?

@stevermeister
Copy link
Owner

let div = document.createElement('div')
div.innerHTML = ngModel;
ngModel = div.textContent

@stevermeister
Copy link
Owner

@bampakoa
Copy link
Contributor Author

I really like your approach. Never thought of it! It is also much more testable. I will make the change and submit a PR for that. Thanks!

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

No branches or pull requests

2 participants