Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 75 additions & 64 deletions homework/list.js
Original file line number Diff line number Diff line change
@@ -1,79 +1,90 @@
'use strict';

(function() {

var ListManager = function(){
this._listNoteContent = [];
this._wrapper = null;
};

var _listNoteContent = [];
var _wrapper = document.querySelector('#note-list-wrapper');
ListManager.prototype=
{
drawList() {
var list = this._listNoteContent;
var ul = document.createElement('ul');
ul.id = 'note-title-list';
var buff = document.createDocumentFragment();
list.forEach(function(note, i) {
var li = document.createElement('li');
li.dataset.noteId = i;
li.classList.add('note-title');
li.textContent = note.title;
// Note: buff is captured, so we now have a
// little closure naturally.
buff.appendChild(li);
});
ul.appendChild(buff);
this._wrapper.appendChild(ul);
},

function start() {
fetchList(function(data) {
updateList(data);
drawList();
preloadFirstNote();
});
window.addEventListener('click', function(event) {
onNoteOpen(event);
});
}

function onNoteOpen(event) {
if (event.target.classList.contains('note-title')) {
var id = event.target.dataset.noteId;
var content = _listNoteContent[id];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
};
}
onNoteOpen(event) {
if (event.target.classList.contains('note-title')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a function is wrapped in a condition statement with only one branch, it's better to move the if...else into the caller, rather than callee.

var id = event.target.dataset.noteId;
var content = this._listNoteContent[id];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
};
},

function preloadFirstNote() {
if (_listNoteContent.length !== 0) {
var content = _listNoteContent[0];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
}
}
preloadFirstNote() {
if (this._listNoteContent.length !== 0) {
var content = this._listNoteContent[0];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
}
},

function updateList(list) {
_listNoteContent = list;
}
updateList(list) {
this._listNoteContent = list;
},

function drawList() {
var list = _listNoteContent;
var ul = document.createElement('ul');
ul.id = 'note-title-list';
var buff = document.createDocumentFragment();
list.forEach(function(note, i) {
var li = document.createElement('li');
li.dataset.noteId = i;
li.classList.add('note-title');
li.textContent = note.title;
// Note: buff is captured, so we now have a
// little closure naturally.
buff.appendChild(li);
});
ul.appendChild(buff);
_wrapper.appendChild(ul);
}
fetchList(afterFetch) {
return new Promise((function(resolve,reject){
var xhr = new XMLHttpRequest();
xhr.open('GET', 'http://127.0.0.1:8000/demo-list-notes.json', true);
xhr.responseType = 'json';
xhr.onreadystatechange = function(e) {
// Watch out: we have a mysterious unknown 'this'.
if (this.readyState === 4 && this.status === 200) {
var listData = this.response;
// The flow ends here.
afterFetch(listData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WRONG: You either keep it as a callback-style function (so you won't get the point) OR re-write it as a Promise-based function (that's what I need you to complete). But to wrap it inside a Promise without resolving or rejecting the result is showing you didn't understand Promise after all. Please, check the MDN link here:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#Examples

Or if you like, you can look up any other examples on the Internet.

} else if (this.status !== 200 ){
// Ignore error in this case.
}
};
xhr.send();
}).bind(this));
},

function fetchList(afterFetch) {
var xhr = new XMLHttpRequest();
xhr.open('GET', 'http://127.0.0.1:8000/demo-list-notes.json', true);
xhr.responseType = 'json';
xhr.onreadystatechange = function(e) {
// Watch out: we have a mysterious unknown 'this'.
if (this.readyState === 4 && this.status === 200) {
var listData = this.response;
// The flow ends here.
afterFetch(listData);
} else if (this.status !== 200 ){
// Ignore error in this case.
}
};
xhr.send();
}
start() {
this.fetchList((function(data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a crucial mistake here, and again shows you did completely not know what I've taught. I believe I already asked all of you lots of times whether you had questions or not, and this error just shows you might need to ask more, if you still don't know why this is a mistake.

this.updateList(data);
this.drawList();
this.preloadFirstNote();
}).bind(this));
window.addEventListener('click', (function(event) {
this.onNoteOpen(event);
}.bind(this)));
}
};

document.addEventListener('DOMContentLoaded', function(event) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't keep this trick used in the (bad) closure example. You need to centralize them into one start-up method.

start();
var listManager = new ListManager();
listManager._wrapper=document.querySelector('#note-list-wrapper');
listManager.start();

});

})();
3 changes: 2 additions & 1 deletion homework/test/test-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ describe('Test > ', function() {
beforeEach(function() {
});

it('will test some pure functions', function() {
it('Math.sin()', function() {
// Write any pure function assertion here.
assert.equal('1',Math.sin(Math.PI/2));
});
});