Skip to content

102502038 #53

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
leemiyinghao wants to merge 2 commits into
base: homework
Choose a base branch
from
Open

102502038 #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a212986
Complete Tier1 part of HW3
leemiyinghao Oct 26, 2015
27e919c
1st modify after HW submit
leemiyinghao Nov 9, 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
66 changes: 35 additions & 31 deletions homework/content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,41 @@
'use strict';

(function() {
var _wrapper = document.querySelector('#note-content-wrapper');
function ToDoItemReader() {
this._wrapper = document.querySelector('#note-content-wrapper');
}
ToDoItemReader.prototype = {
start: function() {
window.addEventListener('note-open', this);
},

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

function resetWrapper() {
_wrapper.innerHTML = '';
}
drawNote: function(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);
}.bind(this));
this._wrapper.appendChild(h);
this._wrapper.appendChild(buff);
},

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);
}
handleEvent: function(event) {
var note = event.detail;
this.resetWrapper();
this.drawNote(note);
}
}
document.addEventListener('DOMContentLoaded', 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 better to put all startup code like this in one method, rather than two native handlers like this.

var toDoItemReader = new ToDoItemReader();
toDoItemReader.start();
});

document.addEventListener('DOMContentLoaded', function(event) {
start();
});
})();
78 changes: 38 additions & 40 deletions homework/karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,65 +2,63 @@
// Generated on Mon Sep 28 2015 19:56:05 GMT+0800 (CST)

module.exports = function(config) {
config.set({
config.set({

// base path that will be used to resolve all patterns (eg. files, exclude)
basePath: '',
// base path that will be used to resolve all patterns (eg. files, exclude)
basePath: '',

// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['mocha', 'chai', 'sinon'],

// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['mocha', 'chai', 'sinon'],

// list of files / patterns to load in the browser
files: [
'*.js',
'test/test-*.js',
],

// list of files / patterns to load in the browser
files: [
'*.js',
'test/test-*.js'
],
// list of files to exclude
exclude: [
],


// list of files to exclude
exclude: [
],
// preprocess matching files before serving them to the browser
// available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
preprocessors: {
},


// preprocess matching files before serving them to the browser
// available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
preprocessors: {
},
// test results reporter to use
// possible values: 'dots', 'progress'
// available reporters: https://npmjs.org/browse/keyword/karma-reporter
reporters: ['progress'],


// test results reporter to use
// possible values: 'dots', 'progress'
// available reporters: https://npmjs.org/browse/keyword/karma-reporter
reporters: ['progress'],
// web server port
port: 9876,


// web server port
port: 9876,
// enable / disable colors in the output (reporters and logs)
colors: true,


// enable / disable colors in the output (reporters and logs)
colors: true,
// level of logging
// possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG
logLevel: config.LOG_DEBUG,


// level of logging
// possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG
logLevel: config.LOG_INFO,
// enable / disable watching file and executing tests whenever any file changes
autoWatch: true,


// enable / disable watching file and executing tests whenever any file changes
autoWatch: true,
// start these browsers
// available browser launchers: https://npmjs.org/browse/keyword/karma-launcher
browsers: ['Firefox'],


// start these browsers
// available browser launchers: https://npmjs.org/browse/keyword/karma-launcher
browsers: ['Firefox'],


// Continuous Integration mode
// if true, Karma captures browsers, runs the tests and exits
singleRun: false
})
// Continuous Integration mode
// if true, Karma captures browsers, runs the tests and exits
singleRun: false,
})
}
150 changes: 77 additions & 73 deletions homework/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,79 +1,83 @@
'use strict';
function ToDoList(){
this._listNoteContent = [];
this._wrapper = document.querySelector('#note-list-wrapper');
}
ToDoList.prototype = {
start: function(){
var promise = this.fetchList();
promise.then(
(data) => {
this.updateList(data);
this.drawList();
if (this._listNoteContent.length !== 0){
this.preloadFirstNote();
}
});
promise.catch(
(reason) => {
throw reason;
}
);
Copy link
Author

@leemiyinghao leemiyinghao Nov 9, 2015

Choose a reason for hiding this comment

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

Not sure if this is what Promise code should be like.
My understanding is pulling callback out from the function.

Copy link
Contributor

@snowmantw snowmantw Nov 15, 2015

Choose a reason for hiding this comment

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

It's unfortunately wrong. You just got trapped by a common mistake. In fact, you should do this if you don't want to use the plain chaining style:

promise = promise.then(....);
promise = promise.catch(....);

Please note it's because every time the then or catch is called, it will create a new instance of then-able. So if you do this:

promise.then(task1)
promise.then(task2)

It will NOT guarantee task2 is only after task1. In fact, they will execute concurrently. So you must make sure the then-able is updated as I wrote.

window.addEventListener('click',this);
},

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

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

var _listNoteContent = [];
var _wrapper = document.querySelector('#note-list-wrapper');
drawList: function() {
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);
});
}
fetchList: function() {
return new Promise(function(resolve, reject){
var xhr = new XMLHttpRequest();
xhr.open('GET', '/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) {
// The flow ends here.
resolve(this.response);
} else if (this.status !== 200 ){
// Ignore error in this case.
reject('Status Error: '+this.status);
}
};
xhr.send();
});
},

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

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

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

})();
};
document.addEventListener('DOMContentLoaded', function(event) {
var toDoList = new ToDoList();
toDoList.start();
});
34 changes: 31 additions & 3 deletions homework/test/test-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,36 @@
'use strict';
describe('Test > ', function() {
beforeEach(function() {
var toDoList, toDoItemReader;
beforeEach(function() {
toDoList = new ToDoList();
toDoItemReader = new ToDoItemReader();
toDoList._wrapper = document.createElement('div');
toDoItemReader._wrapper = document.createElement('div');
});

it('will test some pure functions', function() {
// Write any pure function assertion here.
});
toDoItemReader.start();
toDoList._listNoteContent[0] = {
title: "This is an pen",
passages: [
"This is a pencil",
"That is a book"
]
};
var onNoteOpenEvent = {
target: {
dataset: {
noteId: 0,
},
classList: {
contains: function(input){
return true;
}
}
}
}
Copy link
Author

@leemiyinghao leemiyinghao Nov 9, 2015

Choose a reason for hiding this comment

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

Tried to create a real click event but fail.
Are there any method to set event.target to a fake div object? Or it's not needed?

Copy link
Contributor

@snowmantw snowmantw Nov 15, 2015

Choose a reason for hiding this comment

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

Why did you think you need to create a real click event? I just mentioned that the name is wrong, because in the code it listen to click event, but handle it with the method named as onNoteOpen, not onNoteClicked.

Copy link
Contributor

@snowmantw snowmantw Nov 15, 2015

Choose a reason for hiding this comment

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

And if you need to test a event in unit tests, you just create a plain object as:

{ type: 'click', details: ...}

And then feed it to the handleEvent method. You don't really need to create a click event.

toDoItemReader._wrapper.innerHTML = "";
toDoList.handleEvent(onNoteOpenEvent);
assert.notEqual(toDoItemReader._wrapper.innerHTML, "");
}.bind(this));
});