st-h / ember-content-editable

A contenteditable component for ember-cli that just works™
MIT License
66 stars 31 forks source link

The contenteditable attribute selector is not correct #105

Closed dserv-nh closed 2 years ago

dserv-nh commented 4 years ago

Hello,

In this pull request you added -webkit-user-modify: read-write-plaintext-only; to the [contenteditable] selector.

I think this is not correct because it may have a "false" value. We use TinyMCE which set's this property to false when you want the content to be read only and also on MDN there is a reference to the "false" option. I think you should adjust the selector to only select elements that are truly editable.

st-h commented 4 years ago

@dserv-nh thanks for opening this issue. If I understand you correctly you are suggesting to change the selector to?

[contenteditable="true"] {
   cursor: text;
   -webkit-user-modify: read-write-plaintext-only;
 }
dserv-nh commented 4 years ago

Yes, but you need to be careful because your component only has contenteditable without a value. Therefore the [contenteditable="true"] would not work on this. From MDN:

If the attribute is given without a value, like <label contenteditable>Example Label</label>, its value is treated as an empty string.

I think you should consider using a more specific selector to the ember-content-editable component and not changing this globally for every element with contenteditable so something like .ember-content-editable[contenteditable=""] should be more appropriate

dserv-nh commented 4 years ago

To solve this issue, can you merge this pull request?

st-h commented 4 years ago

sorry, have been very busy with a lot of other stuff. There have been a few minor things I wanted to fix/upgrade as well. Try to get it done asap. Just merged the PR.

st-h commented 2 years ago

sorry about the late reply. This should be now fixed in version 3.0.0. Upgrades require to migrate due to two way binding no longer being available with current ember versions.