Skip to content

added readme - #37

Open
nurhsans wants to merge 26 commits into
wdi-sg:masterfrom
nurhsans:master
Open

added readme#37
nurhsans wants to merge 26 commits into
wdi-sg:masterfrom
nurhsans:master

Conversation

@nurhsans

Copy link
Copy Markdown

flowchart a bit funny still. will edit over weekend

Comment thread index.html
<body>
<script src="js/script.js" charset="utf-8"></script>
<!-- <h1>The PRICE is RIGHT!</h1> -->
<div class="container">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

indentation

Comment thread index.html
</div>

</div>
<!-- <div class = "shoppingCart">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove this if you don't use it anymore

Comment thread assets/js/script.js
@@ -0,0 +1,198 @@
$(function () {

var i = 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is this variable for?

Comment thread assets/js/script.js
$('.productSpace').prepend($productImg)
$('.productSpace').append($printProductName)

console.log(theProduct)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

take the console.log once you're done testing it

Comment thread assets/js/script.js


function submitGuess() {
$('.player1Div').append($('<input id="player-1" type="number">'), $('<input id = "submit-1" type="submit" value="Submit">'))

@primaulia primaulia Oct 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, why don't you prepare this on the html instead? Rather than appending this through the js.

Comment thread assets/js/script.js


$('.result').on('click', '#nextProduct', function (){
if ($( "li" ).length == 3) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you need to keep track the winning flow by the amount of html elements? You shouldn't depend so much on your html to control your logic, similarly your logic shouldn't affect your html a lot too.

@primaulia

Copy link
Copy Markdown
Contributor

Project Workflow: 2 / 5
Technical Requirement: 3 / 5
Creativity: 3 / 5
Code quality: 2 / 5
Problem Solving: 3 / 5
Delivery: 4 / 5
Professional Skill: 4 / 5

Glow

  • Good presentation, you seem comfortable at presenting, keep it up
  • Nice job that you manage to work on what you've understood within time
  • Interesting idea, and good attempt on using JSON

Grow

  • Your project flowchart is illegible, please prepare a bigger flowchart next time
  • Your function flow and variable is not following a good standard. Let's walk through this on the consultation
  • Separation between logic and dom manipulation is not clear, making your code even harder to understand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants