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
76 changes: 42 additions & 34 deletions homework/content.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
'use strict';

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

var ToDoContentManager = function () {
this._wrapper = null;
};
ToDoContentManager.prototype = {
start(){
/*
*Initiate wrapper.
*To do bind(this)
*/
this._wrapper = document.querySelector('note-content-wrapper');
window.addEventListener('note-open',(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.

Rather than binding on an anonymous function, you need to bind the this via set it as the listener of addEventListener.

var note = event.detail;
this.resetWrapper();
this.drawNote(note);
}).bind(this));
},

resetWrapper() {
this._wrapper.innerHTML = '';
},

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);
}
};

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

function resetWrapper() {
_wrapper.innerHTML = '';
}

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);
}

document.addEventListener('DOMContentLoaded', function(event) {
start();
});
})();
exports.ToDoContentManager = ToDoContentManager;

})(window);
1 change: 1 addition & 0 deletions homework/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<meta charset="UTF-8">
<title> Homework - Note List </title>
<script defer src="list.js"></script>
<script defer src="main.js"></script>
<script defer src="content.js"></script>
<link rel="stylesheet" type="text/css" href="main.css" />
</head>
Expand Down
120 changes: 63 additions & 57 deletions homework/list.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,48 @@
'use strict';

(function() {

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

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')) {
(function(exports) {

var ToDoListManager = function(){
this._listNoteContent = [];
this._wrapper = document.querySelector('#note-list-wrapper');
};

ToDoListManager.prototype = {
start(){
this.fetchList()
.then((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.

I recommend you to use Arrow function instead of call bind, since it's more expressive. See:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#Lexical_this

this.updateList(data);
this.drawList();
this.preloadFirstNote();
}).bind(this));
window.addEventListener('click', (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.

It's a convention to prefix on to the corresponding function. So it should be renamed as onClick actually.

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

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

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

function updateList(list) {
_listNoteContent = list;
}

function drawList() {
var list = _listNoteContent;
}
},
updateList(list) {
this._listNoteContent = list;
},
drawList() {
var list = this._listNoteContent;
var ul = document.createElement('ul');
ul.id = 'note-title-list';
var buff = document.createDocumentFragment();
Expand All @@ -52,28 +56,30 @@
buff.appendChild(li);
});
ul.appendChild(buff);
_wrapper.appendChild(ul);
}
this._wrapper.appendChild(ul);
},

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();
}

document.addEventListener('DOMContentLoaded', function(event) {
start();
});
fetchList(afterFetch) {
return new Promise((function(resolve , reject){
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 good to see you have successfully "Promisified" the function. However, please remove the callback argument. It's now useless.

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.
resolve(listData);
} else if (this.status !== 200 ){
// Ignore error in this case.
reject('Error!');
}
};
xhr.send();
}).bind(this));
}

};

})();
exports.ToDoListManager = ToDoListManager;
})(window);
8 changes: 8 additions & 0 deletions homework/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

document.addEventListener('DOMContentLoaded',function(event){
var ToDoContentManager = new ToDoContentManager();
var ToDoListManager = new ToDoListManager();
ToDoContentManager.start();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't overwrite existing classes with the instances. You should rename them as toDoContentManager and toDoListManager.

ToDoListManager.start();
});
8 changes: 6 additions & 2 deletions homework/test/test-list.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
describe('Test > ', function() {
describe('Test ToDoListManager ', function() {
var subject;
beforeEach(function() {
subject = new ToDoListManager();
});

it('will test some pure functions', function() {
// Write any pure function assertion here.
var test = subject;
var sum = test.puerFunction(1,1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is your puerFunction? And a typo?

assert.equal(sum,2,'It is equal to 2!');
});
});