diff --git a/README.md b/README.md index 065ab8e..e1b6dba 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ Simon Learns Chinese is based on the concept of the memory game, Simon, and inspired by gamified language learning apps such as Duolingo. The purpose of the game is to test the user's memory while allowing the user to gain a basic but intuitive understanding of the Chinese language through repeated exposure to simple vocabulary and grammar patterns. Play it here: https://sarah-yu.github.io/simon - + ## Current Features @@ -47,3 +47,6 @@ Play it here: https://sarah-yu.github.io/simon ## Contribute Source code: https://github.com/sarah-yu/simon + + + diff --git a/feedback.md b/feedback.md new file mode 100644 index 0000000..f9ebb26 --- /dev/null +++ b/feedback.md @@ -0,0 +1,18 @@ +# Project 1 Evaluation +[inline code comments]() +## Planning / Process / Submission +**2.5: Performing** +>Submission contains clear evidence of planning, adequate documentation, include all from previous category, as well as additional information such as unsolved issues. + +## Technical Requirements +**2.5: Performing** +>App functions with minimal or no errors and is deployed on github pages + +## Code Quality +**3: Excelling** +>No major code quality issues, makes use of JavaScript best practices appropriately, and follows techniques such as separation of concerns, abstraction, and encapsulation + +## Creativity and Interface +**2.5: Excelling** +>The app is fully responsive, incorporates outside technologies such as Flexbox. App incorporates modern UI themes, and adds unique flair. +> Amazing creative spin on Simon. Only reason I did not give this a 3 was because of sizing issues. Look into media queries! diff --git a/index.html b/index.html index ee8e356..53c901b 100644 --- a/index.html +++ b/index.html @@ -3,6 +3,7 @@ + Simon Learns Chinese @@ -10,8 +11,10 @@
@@ -33,3 +36,5 @@ + + diff --git a/js/script.js b/js/script.js index 73d57c3..5fde167 100644 --- a/js/script.js +++ b/js/script.js @@ -1,4 +1,5 @@ $('document').ready(function() { + // You dont really need document.ready here since your script file is already at the bottom of your html let gameBoard = $('#game-board') let wordBank = $('#word-bank') let startPageEl = $('.start-page') @@ -6,7 +7,9 @@ $('document').ready(function() { let currentLevel = $('#current-level') let scoreEl = $('#score') let score = 0 + let timeToGuess // Declare this at the top to avoid console error when you click `Start Page` const words = [] + // Good job defining your variables at the top of the page words.push(yiGe) words.push(liangGe) @@ -58,6 +61,8 @@ $('document').ready(function() { words.push(fanQie) words.push(huangGua) words.push(hanBaoBao) + // Couple thoughts here. One, consider moving this ^ into it's own seperate js file, or just into word.js + // Also, in word.js, you could put all of the words in the different parts of speech into their own arrays (subjects, verbs, etc). Then you could just run a forEach over each of those arrays, pushing each of the words into the `words` array. const measureWords = words.filter(word => { return word.partOfSpeech === 'measureWord' @@ -78,6 +83,7 @@ $('document').ready(function() { const adjectives = words.filter(word => { return word.partOfSpeech === 'adjective' }) + // Good use of filter, though I think a lot of this could be avoided if you just put each of the parts of speech words in their own arrays to begin with. let level1 = { level: 1, @@ -106,11 +112,12 @@ $('document').ready(function() { startPage() function startPage() { - gameBoard.append(` -
-

Test your memory skills while learning Chinese!

-

Select a level to start the game.

-
`) + gameBoard.append( + `
+

Test your memory skills while learning Chinese!

+

Select a level to start the game.

+
` + ) wordBank.append( `
@@ -120,12 +127,10 @@ $('document').ready(function() {
` ) - $('.level-buttons') - .children() - .on('click', e => { - let userLevel = e.target.id - game(userLevel) - }) + $('.level-buttons').children().on('click', e => { + let userLevel = e.target.id + game(userLevel) + }) } function game(userLevel) { @@ -141,6 +146,7 @@ $('document').ready(function() { break } } + // Good use of swtich statements function startLevel(level) { currentLevel.text(`Level: ${level.level}`) @@ -159,11 +165,9 @@ $('document').ready(function() { function createGameBoard(level) { for (let i = 0; i < level.pos.length; i++) { gameBoard.append(`
`) - $('.the-sentence-pos') - .eq([i]) - .append( - `
${level.posGBLabel[i]}
` - ) + $('.the-sentence-pos').eq([i]).append( + `
${level.posGBLabel[i]}
` + ) } } @@ -171,6 +175,7 @@ $('document').ready(function() { for (let i = 0; i < level.pos.length; i++) { let ptOfSpeech = level.posProperty[i] theSentence[ptOfSpeech] = getWord(level.pos[i]) + // Consider declaring `theSentence` as a global variable at the top of the page. Also I think renaming it something like `currentSentance` or something like that might make a little more sense. } } @@ -182,13 +187,14 @@ $('document').ready(function() { function displayTheSentence(level) { for (let i = 0; i < level.pos.length; i++) { let ptOfSpeech = level.posProperty[i] - $('.the-sentence-pos').eq([i]).prepend(` -
+ $('.the-sentence-pos').eq([i]).prepend( + `
${theSentence[ptOfSpeech] .pinyin} ${theSentence[ptOfSpeech].cn} ${theSentence[ptOfSpeech].en} -
`) +
` + ) } } @@ -209,10 +215,11 @@ $('document').ready(function() { function createWordBank(level) { for (let i = 0; i < level.pos.length; i++) { - wordBank.append(` -
-
${level.posWBLabel[i]}
-
`) + wordBank.append( + `
+
${level.posWBLabel[i]}
+
` + ) $('.wb-pos-label').append('
') } @@ -239,8 +246,7 @@ $('document').ready(function() { window['wb' + ptOfSpeech][0] = theRightWord window['shuffleWb' + ptOfSpeech] = level.pos[i].slice() window['shuffleWb' + ptOfSpeech].splice( - window['shuffleWb' + ptOfSpeech].indexOf(theRightWord), - 1 + window['shuffleWb' + ptOfSpeech].indexOf(theRightWord), 1 ) getRandomWords(window['shuffleWb' + ptOfSpeech], 3) @@ -255,6 +261,7 @@ $('document').ready(function() { } displayWBTiles(level) } + // Hmm, you shouldn't need to read or write anything on the window object. Keep your current wordbank in your JS file and update it for each question. // reference: https://stackoverflow.com/questions/11935175/sampling-a-random-subset-from-an-array function getRandomWords(array, size) { @@ -324,6 +331,7 @@ $('document').ready(function() { }) } } + // Consider breaking userGuess up into 2 or 3 smaller more specific funtions function checkSentence(level) { if (theSentence.isEqual(userSentence)) { @@ -366,3 +374,16 @@ $('document').ready(function() { scoreEl.text(0) }) }) + + +// Great job on the project! Your approach to functional programming is very solid and well organized. Most functions are small and precise. There are a few spots that I mentioned above that could improve the quality of your code, the biggest ones being... +// - Pushing the individual words into an array could be done more systematically and logic could really be abstracted to another file. +// - You shouldn't need to manipulate or read data through the window object. Keep data local and update the page through HTML as needed. + +// Here are a few other suggestions/features to make moving forward... +// - Something to happen when a user makes an incorect guess, or a `next question` button (If I have no idea what the sentence is on level 3, the only way to move forward is by guessing a ton of words) +// - Maybe a submit button for each sentence +// - Have the timer count down from 5 to 0 +// - Change `Start Page` button to `Start Menu` or something like that. + +// Overall this is a very strong project and you really went above and beyond. I can see something like serving a real need in the real word. Great job! diff --git a/js/sentence.js b/js/sentence.js index daa5b87..cbb758f 100644 --- a/js/sentence.js +++ b/js/sentence.js @@ -22,6 +22,7 @@ class Sentence { checkLevelThreeSentence() { return this.checkLevelTwoSentence() && userSentence.adjective != null } + // Nice way of keeping things concise! ^ isEqual(userSentence) { let correct = false @@ -53,3 +54,6 @@ class Sentence { let theSentence = new Sentence() let userSentence = new Sentence() +// Declare these in the script.js file to make script.js easier to follow along with + +// Great use of classes here. Methods are well defined. You could arguably abstract some of the isEqual method into smaller more defined methods, but great job overall here! diff --git a/js/word.js b/js/word.js index 17082f7..df2903c 100644 --- a/js/word.js +++ b/js/word.js @@ -6,6 +6,7 @@ class Word { this.partOfSpeech = partOfSpeech } } +// Nice use of a class here! // partOfSpeech: measureWord const yiGe = new Word('一个', 'one', 'yí gè', 'measureWord') @@ -18,6 +19,7 @@ const qiGe = new Word('七个', 'seven', 'qī gè', 'measureWord') const baGe = new Word('八个', 'eight', 'bā gè', 'measureWord') const jiuGe = new Word('九个', 'nine', 'jiǔ gè', 'measureWord') const shiGe = new Word('十个', 'ten', 'shí gè', 'measureWord') +// Talked more about this in script.js, but consider putting the parts of speech into their own arrays to make data easier to manipulate. // partOfSpeech: subject const wo = new Word('我', 'I', 'wǒ', 'subject') diff --git a/style.css b/style.css index 0e80b5f..25b2d47 100644 --- a/style.css +++ b/style.css @@ -2,6 +2,7 @@ html, body { color: #3a3a3b; font-family: 'Karla', sans-serif; + /*Consider using an additional backup font between `Karla` and `san-serif`*/ height: 100%; } @@ -64,6 +65,8 @@ header button:hover { .site-logo { font-size: 2vw; + /*I think it makes sense to make the font-size here fixed, since the title is the only thing that is changing sizes when the page gets smaller*/ + /*Also, consider calling this `title` instead or even better, just using a heading*/ font-weight: 700; margin-left: auto; margin-right: auto; @@ -211,3 +214,9 @@ main { color: #fff; background: #cd6155; } + +/*Good job keeping things well organized and easy to read. You've chosen good semantic class/id names to make things intuitive to understand. Typically with CSS you'll want to go from least specific at the top to most specific at the bottom, so (elements, then classes, then ids). However, since you've provided commented titles, this is also an acceptable way to do it. For larger CSS files, and yours is starting to verge in that territory, consider breaking up the sections (header, main, wordbank, etc) into their own seperate CSS files.*/ + +/*Sizing is good to a point. Nice work making use of flex-box. For more dynamic sizing, look into using *media queries* that can reformat and apply different CSS rules when your page gets smaller than a certain size. https://www.w3schools.com/css/css_rwd_mediaqueries.asp This could really help your page when it the screen size becomes small. */ + +/*Aesthetically speaking, you've done a great job choosing the color palette, using rounded buttons, making use of `hover`, and chosing suitable font styling and size. Consider adding a small transition time when user hovers over a button. Page is pleasing to look at, easy to understand what's going on, and not too busy. */