-
Notifications
You must be signed in to change notification settings - Fork 129
#469: PoC #470
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
base: master
Are you sure you want to change the base?
#469: PoC #470
Conversation
…teditable="plaintext-only"
|
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 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 |
|
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: 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 I did find a small issue in the new switch statement. It only toggles between: The case for case "true":
newMode = "plaintext-only";
break; to case true:
newMode = "plaintext-only";
break; the loop started working properly. |
|
Yes, sorry, the second And |
|
No problem, I thought you might have added additional changes. As far as I'm concerned, the changes can be merged/incorporated into your version. Thank you for your help! |
In order to have the
contenteditablelogic in one place, I moved check forcontenteditable === TRUEto its own helper function and added a check forplaintext-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?