Web-based text editor (#2)

Question

var appname = " - simple notepad",
 textarea = document.getElementById("textarea"),
 untitled = "untitled.txt" + appname,
 filename = "*.txt",
 isModified;
document.title = untitled;
textarea.onpaste = textarea.onkeypress = function() {
 isModified = true;
};
function confirmNav() { // Confirm navigation
 if (isModified) {
 var a = window.confirm("You have unsaved changes that will be lost.");
 if (a) {
 isModified = false;
 }
 }
}
function New() { // New
 confirmNav();
 if (!isModified) {
 textarea.value = "";
 document.title = untitled;
 }
 textarea.focus();
}
function Open() { // Open
 confirmNav();
 if (!isModified) {
 document.getElementById("selected_file").click();
 }
}
function loadFileAsText() { // load file as text
 var selected_file = document.getElementById("selected_file").files[0];
 var fileReader = new FileReader();
 fileReader.onloadend = function(e) {
 if (e.target.readyState === FileReader.DONE) {
 filename = selected_file.name;
 document.title = filename + appname;
 textarea.value = e.target.result;
 textarea.focus();
 }
 };
 fileReader.readAsText(selected_file);
}
function rename() { // Rename
 filename = window.prompt("Name this note:", filename);
 if (filename) {
 filename = (filename.lastIndexOf(".txt") === -1) ? filename + ".txt" : filename;
 document.title = filename + appname;
 }
}
function Save() { // Save
 rename();
 if (filename) {
 var blob = new Blob([textarea.value], {
 type: "text/plain;charset=utf-8"
 });
 window.saveAs(blob, filename);
 isModified = false;
 }
 textarea.focus();
}
function Print() { // Print
 var prnt_helper = document.getElementById("prnt_helper");
 prnt_helper.innerHTML = textarea.value;
 window.print();
 prnt_helper.innerHTML = "";
 textarea.focus();
}
function Help() { // Help
 var help = document.getElementById("help_content"),
 overlay = document.getElementById("overlay");
 function closeHelpAndOverlay() {
 help.setAttribute("aria-hidden", "true");
 overlay.setAttribute("aria-hidden", "true");
 textarea.focus();
 }
 if (help["aria-hidden"] === "true") {
 closeHelpAndOverlay();
 } else {
 help.setAttribute("aria-hidden", "false");
 overlay.setAttribute("aria-hidden", "false");
 textarea.blur();
 document.getElementById("overlay").onclick = function() {
 closeHelpAndOverlay();
 };
 // document.onkeydown = function(e) { // esc to close help
 // if (e.keyCode === 27 || e.which === 27) {
 // closeHelpAndOverlay();
 // }
 // };
 }
}
// Confirm close
window.onbeforeunload = function() {
 if (isModified) {
 return "You have unsaved changes that will be lost.";
 }
};
// Keyboard shortcuts
document.onkeydown = function(e) {
 var key = e.keyCode || e.which;
 if (e.ctrlKey) {
 if (e.altKey) {
 if (key === 78) {
 // Ctrl+Alt+N
 New();
 }
 }
 if (key === 79) {
 // Ctrl+O
 e.preventDefault();
 Open();
 }
 if (key === 83) {
 // Ctrl+S
 e.preventDefault();
 Save();
 }
 if (key === 80) {
 // Ctrl+P
 e.preventDefault();
 Print();
 }
 if (key === 191) {
 // Ctrl+/
 e.preventDefault();
 Help();
 }
 }
 if (e.keyCode === 9 || e.which === 9) { // tab
 e.preventDefault();
 var s = textarea.selectionStart;
 textarea.value = textarea.value.substring(0, textarea.selectionStart) + "\t" + textarea.value.substring(textarea.selectionEnd);
 textarea.selectionEnd = s + 1;
 }
};

• whitehat101: Do you have a question, or any specific part of your code you want looked at? Or just anything and everything?
• John Dvorak: @Jeremy I'll assume "anything and everything"
• udb: yea you can say anything, the whole code, the way I coded this stuff. you may suggest me some simpler way if there are... and so on. thanks.

Answer 1

You can optimize some queries.

var selected_file = document.getElementById("selected_file")

Unless your HTML changes considerably (replacing nodes periodically), every call to getElementById() will return the same node. You can access the node more efficiently if you cache the node in a local variable, and use the same local variable throughout your code, instead of looking it up multiple times.

Same with:

// Help
var help = document.getElementById("help_content"),
 overlay = document.getElementById("overlay");

As long as all these elements exist when your code initializes, you can lookup and store them in variables once, instead of each time Help() (etc) is called. In your code, the performance increase will be very minor, but it can add up in other situations.

---

In your keydown block:

if (key === 79) {
...
if (key === 83) {
...
// function rename()
(filename.lastIndexOf(".txt") === -1)

You can shave a few bytes by using == instead of ===. Since you're comparing primitives, not objects, == and === function equivalently.

Using === (instead of ==) is a good practice when you're not sure, but it is unnecessary in the places you are using it.

---

It looks like you didn't update your last check in the keydown callback

if (e.keyCode === 9 || e.which === 9) { // tab

Can be rewritten as:

if (key == 9) { // tab

---

function confirmNav() { // Confirm navigation
 if (isModified) {
 var a = window.confirm("You have unsaved changes that will be lost.");
 if (a) {
 isModified = false;
 ...

---

While nothing is technically wrong here, I'd either like to see the confirm inlined into the if() or a more descriptive variable name. a is meaningless.

 if (window.confirm("You have unsaved changes that will be lost.")) {
 // or 
 var leavePage = window.confirm("You have unsaved changes that will be lost.");
 if (leavePage) {

---

Your code only has three long lines. The longest, at 136 chars:

var s = textarea.selectionStart
textarea.value = textarea.value.substring(0, textarea.selectionStart) + "\t" + textarea.value.substring(textarea.selectionEnd);
textarea.selectionEnd = s + 1;

I compacted it be using the local you already declared, and by adding a second. Now 97 chars:

var sStart = textarea.selectionStart,
 txt = textarea.value;
textarea.value = txt.substring(0, sStart) + "\t" + txt.substring(textarea.selectionEnd);

I don't think s was a bad variable name (s for selection or start is meaningful), but sStart is more clear.

---

You wrote some lovely, readable, and coherent JavaScript.

Here is everything:

var appname = " - simple notepad",
 textarea = document.getElementById("textarea"),
 help = document.getElementById("help_content"),
 overlay = document.getElementById("overlay"),
 selected_file = document.getElementById("selected_file"),
 prnt_helper = document.getElementById("prnt_helper"),
 untitled = "untitled.txt" + appname,
 filename = "*.txt",
 isModified;
document.title = untitled;
textarea.onpaste = textarea.onkeypress = function() {
 isModified = true;
};
function confirmNav() { // Confirm navigation
 if (isModified) {
 var leavePage = window.confirm("You have unsaved changes that will be lost.");
 if (leavePage) {
 isModified = false;
 }
 }
}
function New() { // New
 confirmNav();
 if (!isModified) {
 textarea.value = "";
 document.title = untitled;
 }
 textarea.focus();
}
function Open() { // Open
 confirmNav();
 if (!isModified) {
 selected_file.click();
 }
}
function loadFileAsText() { // load file as text
 var file = selected_file.files[0],
 fileReader = new FileReader();
 fileReader.onloadend = function(e) {
 if (e.target.readyState == FileReader.DONE) {
 filename = file.name;
 document.title = filename + appname;
 textarea.value = e.target.result;
 textarea.focus();
 }
 };
 fileReader.readAsText(file);
}
function rename() { // Rename
 filename = window.prompt("Name this note:", filename);
 if (filename) {
 filename = (filename.lastIndexOf(".txt") == -1) ? filename + ".txt" : filename;
 document.title = filename + appname;
 }
}
function Save() { // Save
 rename();
 if (filename) {
 var blob = new Blob([textarea.value], {
 type: "text/plain;charset=utf-8"
 });
 window.saveAs(blob, filename);
 isModified = false;
 }
 textarea.focus();
}
function Print() { // Print
 prnt_helper.innerHTML = textarea.value;
 window.print();
 prnt_helper.innerHTML = "";
 textarea.focus();
}
function Help() { // Help
 function closeHelpAndOverlay() {
 help.setAttribute("aria-hidden", "true");
 overlay.setAttribute("aria-hidden", "true");
 textarea.focus();
 }
 if (help["aria-hidden"] == "true") {
 closeHelpAndOverlay();
 } else {
 help.setAttribute("aria-hidden", "false");
 overlay.setAttribute("aria-hidden", "false");
 textarea.blur();
 overlay.onclick = function() {
 closeHelpAndOverlay();
 };
 // document.onkeydown = function(e) { // esc to close help
 // if (e.keyCode == 27 || e.which == 27) {
 // closeHelpAndOverlay();
 // }
 // };
 }
}
// Confirm close
window.onbeforeunload = function() {
 if (isModified) {
 return "You have unsaved changes that will be lost.";
 }
};
// Keyboard shortcuts
document.onkeydown = function(e) {
 var key = e.keyCode || e.which;
 if (e.ctrlKey) {
 if (e.altKey) {
 if (key == 78) {
 // Ctrl+Alt+N
 New();
 }
 }
 if (key == 79) {
 // Ctrl+O
 e.preventDefault();
 Open();
 }
 if (key == 83) {
 // Ctrl+S
 e.preventDefault();
 Save();
 }
 if (key == 80) {
 // Ctrl+P
 e.preventDefault();
 Print();
 }
 if (key == 191) {
 // Ctrl+/
 e.preventDefault();
 Help();
 }
 }
 if (key == 9) { // tab
 e.preventDefault();
 var sStart = textarea.selectionStart,
 txt = textarea.value;
 textarea.value = txt.substring(0, sStart) + "\t" + txt.substring(textarea.selectionEnd);
 textarea.selectionEnd = sStart + 1;
 }
};

• udb: Thank you very very much for such a detailed suggestions and advise... I really appreciate it...
• udb: and I'd glad if you take a look at the complete app - dl.dropboxusercontent.com/u/92126558/projects/ntpd/notepad.html
• whitehat101: Nice, that's a very clean app. I'd recommend other ways to dismiss the Help dialog, esc, or an x in the corner. Now I see why you have the esc code commented. You can move the closeHelpAndOverlay() out of Help() to a top level function, then you can add another if at the end of your main keydown, to handle esc and call closeHelpAndOverlay().