Skip to content

Homework #46

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.

Sign up for GitHub

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

Open
hsuehkuan-lu wants to merge 7 commits into
base: homework
Choose a base branch
from
Open

Homework #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99e3f89
test
Oct 16, 2015
67eea32
test done
Oct 16, 2015
2efc149
Deal with object properties w/ constructor & prototype
Oct 28, 2015
cf47814
Modify constructor and prototype & set bind
Oct 30, 2015
197337e
add Main manager
Oct 30, 2015
82daf8e
Deal asynchronous flows with Promise
Oct 30, 2015
7fa5473
add test case for a pure function
Oct 30, 2015
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
69 changes: 38 additions & 31 deletions homework/content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,44 @@
'use strict';

(function() {
var _wrapper = document.querySelector('#note-content-wrapper');
var ContentManager =(function() {

function start() {
window.addEventListener('note-open', function(event) {
var note = event.detail;
resetWrapper();
drawNote(note);
});
}
// constructor
var self = function () {
Copy link
Contributor

@snowmantw snowmantw Nov 7, 2015

Choose a reason for hiding this comment

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

It may not need to create a inner function as the constructor, while the ContentManager is the constructor. And to overwrite the keyword self is not so good.

this._wrapper = null;
};

// prototype
self.prototype = {

function resetWrapper() {
_wrapper.innerHTML = '';
}
start() {
this._wrapper = document.querySelector('#note-content-wrapper');
Copy link
Contributor

@snowmantw snowmantw Nov 7, 2015

Choose a reason for hiding this comment

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

Instead of using an anonymous function, setting the listener as this and handle events with handleEvent method is better.

window.addEventListener('note-open', (function(event) {
var note = event.detail;
this.resetWrapper();
this.drawNote(note);
}).bind(this));
},

function drawNote(note) {
var title = note.title;
var h = document.createElement('h2');
h.textContent = title;
var passages = note.passages;
var buff = document.createDocumentFragment();
passages.forEach(function(passage) {
var p = document.createElement('p');
p.classList.add('note-passage');
p.textContent = passage;
buff.appendChild(p);
});
_wrapper.appendChild(h);
_wrapper.appendChild(buff);
}
resetWrapper() {
this._wrapper.innerHTML = '';
},

document.addEventListener('DOMContentLoaded', function(event) {
start();
});
})();
drawNote(note) {
var title = note.title;
var h = document.createElement('h2');
h.textContent = title;
var passages = note.passages;
var buff = document.createDocumentFragment();
passages.forEach(function(passage) {
var p = document.createElement('p');
p.classList.add('note-passage');
p.textContent = passage;
buff.appendChild(p);
});
this._wrapper.appendChild(h);
this._wrapper.appendChild(buff);
}

};
return self;
})(window);
1 change: 1 addition & 0 deletions homework/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<title> Homework - Note List </title>
<script defer src="list.js"></script>
<script defer src="content.js"></script>
<script src="main-manager.js"></script>
<link rel="stylesheet" type="text/css" href="main.css" />
</head>
<body>
Expand Down
155 changes: 88 additions & 67 deletions homework/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,79 +1,100 @@
'use strict';

(function() {
var ListManager = (function() {

// constructor
var self = function() {
this._listNoteContent = [];
this._wrapper = null;
};

var _listNoteContent = [];
var _wrapper = document.querySelector('#note-list-wrapper');
// prototype
self.prototype = {

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

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 }));
};
}
this._wrapper = document.querySelector('#note-list-wrapper');
this.fetchList()
.then(this.handler.bind(this));

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

@snowmantw snowmantw Nov 7, 2015

Choose a reason for hiding this comment

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

It's a convention to prefix method with on for the corresponding function, so it should be renamed as 'onClick'. And, to set this as the listener is better than an anonymous function.

this.onNoteOpen(event);
}).bind(this));
},

function updateList(list) {
_listNoteContent = list;
}
handler(data){
this.updateList(data);
this.drawList();
this.preloadFirstNote();
},

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);
}
onNoteOpen(event) {
if (event.target.classList.contains('note-title')) {
var id = event.target.dataset.noteId;
var content = this._listNoteContent[id];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
}
},

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.
preloadFirstNote() {
if (this._listNoteContent.length !== 0) {
var content = this._listNoteContent[0];
window.dispatchEvent(new CustomEvent('note-open',
{ detail: content }));
}
};
xhr.send();
}
},

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

document.addEventListener('DOMContentLoaded', function(event) {
start();
});
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);
},

fetchList(afterFetch) {
Copy link
Contributor

@snowmantw snowmantw Nov 7, 2015

Choose a reason for hiding this comment

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

Good, you have successfully "Promisified" it.

return new Promise(function(resolve, reject) {
var xhr = new XMLHttpRequest();
var url = 'http://127.0.0.1:8000/demo-list-notes.json';
xhr.open('GET', url, true);
xhr.responseType = 'json';
xhr.onreadystatechange = ((function(e) {
// Watch out: we have a mysterious unknown 'this'.
// here "this" is a xmlhttprequest = xhr -> so bind xhr
if (this.readyState === 4 && this.status === 200) {
var listData = this.response;
// The flow ends here.
resolve(listData);
} else if (this.status !== 200 ){
// Ignore error in this case.
reject("Error");
}
}).bind(xhr));
xhr.send();
});
},

// test function here
// test num1 add num2
testFunction(num1, num2) {
return num1 + num2;
}
};
return self ;

})();
})(window);
8 changes: 8 additions & 0 deletions homework/main-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

document.addEventListener('DOMContentLoaded', function(event) {
var contentManager = new ContentManager();
var listManager = new ListManager();
contentManager.start();
listManager.start();
});
10 changes: 8 additions & 2 deletions homework/test/test-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
describe('Test > ', function() {
describe('Test ListManager ', function() {
var listManager;
beforeEach(function() {
listManager = new ListManager()
});

it('will test some pure functions', function() {
it('will test add function', function() {
// Write any pure function assertion here.
var num1 = 2;
var num2 = 10;
var sum = listManager.testFunction(num1, num2);
assert.equal(sum, 12, "Equal to 12");
});
});