BorisMoore
#469: PoC
In order to have the contenteditable logic in one place, I moved check for contenteditable === TRUE to its own helper function and added a check for plaintext-only.
As I am not completely familiar with the project, I am unsure which files I need to update. I see a lot of duplication when searching for contenteditable. I guess some files are generated?
Yes, I think this will address the issue.
There is a whole build process on the https://github.com/BorisMoore/jsviews.com repository, so yes these files are actually generated (driven largely by gulp.js). So I think I will need to integrate your changes in that context, rather than directly merging your pull request here.
Let me know if you have any concerns. Thank you for your work... Much appreciated.
I just came across an issue where contenteditable="false" (changed by data binding) also seems to fail. I still have to check whether it is something I did wrong or it is a bug in JsViews.
Maybe the check needs to be changed to include all 3 cases.
Apart from that I have no issues for you to integrate these changes yourself without merging my PR. It is the end result that counts.
Ok, I have updated my minimal example in #469 and found that contenteditable="false" does work as expected.
However, when I try to data-link contenteditable to a property in the model, the changes made in the control do not seem to be bound to the model.
The binding issue you are seeing arises from the processing model, which binds from left to right. You want the contenteditable to update first, followed by content - which needs to be updated based on the already updated contenteditable mode:
So you need this:
<div data-link="contenteditable{:mode} {:text:}"></div>`
rather than this:
<div data-link="{:text:} contenteditable{:mode}"></div>
Thanks for your improved test page!
Incidentally there are a few possible values of contenteditable: "true", "", null, true, "plaintext-only", "false", false
So here is your test page augmented with additional values. It looks to me that they are all functioning correctly.
<html>
<head>
<script type="text/javascript" src="https://code.jquery.com/jquery-3.7.1.js"></script>
<script src="../download/ContentEditableFix.js"></script>
<style>
div[contenteditable] {
width: 400px;
height: 150px;
margin-bottom: 5px;
padding: 2px;
border: solid black 1px;
overflow: auto;
}
</style>
</head>
<body>
<div id="container"></div>
<script id="template" type="text/x-jsrender">
<button data-link="{on toggleMode}">Change mode</button>
contenteditable={^{>mode==""+mode?'"':""}}{^{>mode}}{^{>mode==""+mode?'"':""}}
<div id="ce" data-link="contenteditable{:mode} {:text:}"></div>
<hr/>
contenteditable="true"
<div contenteditable="true" data-link="text"></div>
contenteditable=""
<div contenteditable="" data-link="text"></div>
contenteditable
<div contenteditable data-link="text"></div>
contenteditable=true
<div contenteditable=true data-link="text"></div>
contenteditable="plaintext-only"
<div contenteditable="plaintext-only" data-link="text"></div>
contenteditable="false"
<div contenteditable="false" data-link="text"></div>
contenteditable=false
<div contenteditable=false data-link="text"></div>
</script>
<script type="text/javascript">
var model = {
text: 'some <span style="color: red; font-family: Georgia;">red</span> text',
mode: 'true',
toggleMode: () => {
let newMode = "true";
switch (model.mode) {
case "true":
newMode = "";
break;
case "":
newMode = null;
break;
case null:
newMode = true;
break;
case "true":
newMode = "plaintext-only";
break;
case "plaintext-only":
newMode = "false"
break;
case "false":
newMode = false;
break;
case false:
newMode = "true";
break;
}
console.log("toggle", model, newMode);
$.observable(model).setProperty("mode", newMode);
console.log("toggle", model, newMode);
}
};
$.templates('#template').link('#container', model);
</script>
</body>
</html>
Hi Boris,
Thank you for your explanation of the binding order. I should be able to fix my issue with it.
I think you are right in regards to the extra possible values. I used mdn as a source. It does not list these extra values in the bullet points, but mentions them in text. I missed these the first time around. Anyways, it is best to try to handle all possible (valid) cases, as you have done in your example, so it should not break again.
Unfortunately I am unable to verify your fix, as I cannot find ContentEditableFix.js. You have used a relative path for its src.
I have tried looking for the file in
https://github.com/BorisMoore/jsviews
and
https://github.com/BorisMoore/jsviews.com
but was unable to find it.
I did find a small issue in the new switch statement. It only toggles between:
"true", "", null and true.
The case for "true" is listed twice and the case for true is missing.
When I changed
case "true":
newMode = "plaintext-only";
break;
to
case true:
newMode = "plaintext-only";
break;
the loop started working properly.
Yes, sorry, the second case "true": was a typo - it should have been case true:, as you say.
And ContentEditableFix.js is simply my local copy of jsviews.js incorporating your proposed fix. Sorry not to have been clear on that....
No problem, I thought you might have added additional changes. I have tested the extended test page with my changes and can confirm it works for me too.
As far as I'm concerned, the changes can be merged/incorporated into your version.
Thank you for your help!