Online notepad in JavaScript

Question

I have been told for my code below i need to "Improve and optimise the codebase of the application to reflect modern and best coding practices". Can anyone help me in doing this? As I know the service worker code can't be optimised so I am not sure what other code I can change/improve for best coding practices. The HTML page attached to this is mainly just buttons.

//Make IE play nice with HTML5 elements
document.createElement('header');
document.createElement('section');
document.createElement('article');
document.createElement('footer');
// ServiceWorker is a progressive technology. Ignore unsupported browsers
if ('serviceWorker' in navigator) {
 console.log('CLIENT: service worker registration in progress.');
 navigator.serviceWorker.register('/service-worker.js').then(function() {
 console.log('CLIENT: service worker registration complete.');
 }, function() {
 console.log('CLIENT: service worker registration failure.');
 });
} else {
 console.log('CLIENT: service worker is not supported.');
}
var notepad = {
	notes:[]
};
var currentNoteKey,
	viewEdit,
	viewList,
	noteTitleElement,
	noteContentElement;
function createNote(){
	//Create a blank new note
	var newNote = {
		'name':'New Note',
		'content':'Hello World',
		'last-modified':+new Date()
	}
	//Add the note to the array, and keep track of it's key
	var newNoteKey = 0;
	while(typeof notepad.notes[newNoteKey] !== 'undefined'){
		newNoteKey++;
	}
	currentNoteKey = newNoteKey;
	notepad.notes[newNoteKey] = newNote;
	//Redraw the list of notes, and show the edit view
	drawNotesList();
	changeView();
}
function drawNotesList(){
	//Sort the notes by most recently modified first
	notepad.notes.sort(function(a,b){
		if(a['last-modified'] < b['last-modified']){
			return 1;
		}else if(a['last-modified'] > b['last-modified']){
			return -1;
		}else{
			return 0;
		}
	});
	
	//Generate & Apply the HTML
	var notesList = '';
	for(key in notepad.notes){
		var thisNote = notepad.notes[key];
		notesList += '<li><a href="#" data-key="' + key + '" onclick="noteListClick(this)">' + thisNote.name + '</a></li>';
	}
	if(notesList == ''){
		notesList = '<li class="info">No notes yet</li>';
	}
	document.getElementById('notes-list').innerHTML = notesList;
}
function noteListClick(link){
	currentNoteKey = link.getAttribute('data-key');
	changeView();
}
function changeView(){
	//Used to change between note list and note edit views
	if(viewEdit.className === 'visible'){
		//Transitioning to the Notes List view
		viewEdit.className = '';
		viewList.className = 'visible';
		currentNoteKey = null;
		drawNotesList();
	}else{
		//Transitioning to the Note Edit view
		if(populateEditView()){
			viewEdit.className = 'visible';
			viewList.className = '';
		}
	}
}
function saveNote(){
	notepad.notes[currentNoteKey].content = noteContentElement.value;
	notepad.notes[currentNoteKey]['last-modified'] = +new Date();
}
function populateEditView(){
	if(typeof currentNoteKey === 'undefined' || typeof notepad.notes[currentNoteKey] === 'undefined'){
		alert("Oops, can't find that note!");
		return false;
	}
	noteTitleElement.innerHTML = notepad.notes[currentNoteKey].name;
	noteContentElement.value = notepad.notes[currentNoteKey].content;
	return true;
}
function renameNote(){
	var newName = prompt("New note name:", notepad.notes[currentNoteKey].name);
	notepad.notes[currentNoteKey].name = newName;
	notepad.notes[currentNoteKey]['last-modified'] = +new Date();
	noteTitleElement.innerHTML = newName;
	
}
function deleteNote(){
	if(typeof currentNoteKey !== 'undefined' && typeof notepad.notes[currentNoteKey] !== 'undefined' && confirm("Are you sure?")){
		delete notepad.notes[currentNoteKey];
		changeView();
	}
}
window.onload = function(){
	//Grab references to DOM elements
	viewEdit = document.getElementById('view-edit');
	viewList = document.getElementById('view-list');
	noteTitleElement = document.getElementById('note-title');
	noteContentElement = document.getElementById('note-content');
	//Set up events
	document.getElementById('btn-new').onclick = createNote;
	document.getElementById('btn-back').onclick = changeView;
	document.getElementById('btn-rename').onclick = renameNote;
	document.getElementById('btn-delete').onclick = deleteNote;
	document.getElementById('note-content').onkeyup = saveNote;
	
}

<!doctype html>
<html><head>
		<title>Notepad (Online)</title>
 
		<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1" />
		<link rel="stylesheet" href="notepad.css" />
		
 
	</head>
	<body>
 
		<section id="view-list" class="visible">
			<header>
				<button id="btn-new" class="right">New</button>
 <a href="login.html">
 <button id="btn-login" class="right">Login</button>
 </a>
				<h1>Notepad</h1>
			</header>
			<ul id="notes-list">
				<li class="info">No notes yet</li>
				<noscript>
					<li><a href="http://enable-javascript.com"><strong>Unfortunately, you'll need JavaScript enabled to use Notepad</strong></a></li>
				</noscript>
			</ul>
		</section>
		<section id="view-edit">
			<header>
				<button id="btn-back">Back</button>
				<h1 id="note-title">Loading Note...</h1>
			</header>
			<textarea id="note-content">Loading...</textarea>
			<footer>
				<button id="btn-rename">Save Note</button>
				<button id="btn-delete">Delete</button>
			</footer>
		</section>
 <script src="notepad.js"></script>
 <script src="service-worker.js"></script>
		
 <script src="localstorage.js"></script>
 
	</body>
</html>

• Bálint: Avoid global variables and use closure.
• skyrimveteran: can you give me an example on this?

Answer 1

Javascript developers don't like to put hardly anything on the global scope.

To achieve this, surround your script in an auto running function.

(function scope() {
// your variables and functions go here
})();

If you really need to put something in the global scope (there are only very few cases where this applies), put it inside an object:

var globalVariables = {
 variable1: 1,
 variable2: []
}

Then you can acces it by doing

globalVariables.variable1; // returns 1

When you want to access a variable in the closure, return a function from it.

 var sayHello = (function() {
 var closureVariable = "hello";
 return function() {return closureVariable;};
 })();
 sayHello(); //returns hello

Side note: DOCTYPE should be uppercase

• Pimgd: I think the way to use variables as you describe is good, but your reasons for doing so is bad, javascript in browser already runs in a client, so you are running in a hostile environment from the start anyway (people could edit the code they download from your server)
• Bálint: @Pimgd but not on runtime.
• Bálint: @Quill removed.
• Pimgd: @Bálint runtime editing - stackoverflow.com/questions/7343276/… it's pretty easy - comes as a standard feature in chrome.
• Dan: You can already modify variables at runtime through a breakpoint, even if they're not on the global scope. The main reason we don't like the global scope is because, like every other language, it's really hard to test and bad at encapsulation. Additionally, if you can access something from a closure, you can almost always pass it through as a function argument.