Skip to content

Add flowchart#20

Open
matthewfrancisong wants to merge 17 commits into
wdi-sg:masterfrom
matthewfrancisong:master
Open

Add flowchart#20
matthewfrancisong wants to merge 17 commits into
wdi-sg:masterfrom
matthewfrancisong:master

Conversation

@matthewfrancisong
Copy link
Copy Markdown

No description provided.

Comment thread assets/js/1stdraft.js
@@ -0,0 +1,97 @@
$(function () {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You may delete draft files.

Comment thread assets/js/script.js Outdated

// Create audio event
var $soundButton = $('.sound')
$soundButton.on('click' , playAudio)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You may gather eventListeners together with comments.

Comment thread assets/js/script.js
$soundButton.on('click' , playAudio)

function playAudio () {
document.getElementsByClassName('audio')[0].play()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How can we do this in jQuery way?

Comment thread assets/js/script.js Outdated
}

function gameOver () {
if (totalStepsTakenByP1 >= 100 || totalStepsTakenByP2 >= 100) { whoWon() }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need brackets if you want to do in 1 line.

Comment thread assets/js/script.js Outdated

// reverse the numbering for even or odd rows.
$row.each(function () {
if (Number(($(this).attr('class'))) % 2 === 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I expect if we remove Number(), it will still work, why?

Comment thread assets/js/script.js
painOrPleasureP2()
$circle.css("left", "25px") // indicate player2 turn

if (randomDiceResult === 6) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread assets/js/script.js Outdated
startPosition()

// Using Array to input Snakes & Ladders
var painAndPleasureArray = [0,'ladder0',0,'ladder1',0,0,'snake0',0,'ladder2',0, //row 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we try this with Object, how we can do this?

Comment thread assets/js/script.js
,'ladder9',0,'snake6',0,'snake7',0,0,0,'snake8',0] //row 10


// Player 1 Snakes & Ladders
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The two functions are looks very similar. How can we merge the functions into one painOrPlearsure? Refactor this will be helpful to you.

Hint : you can use parameters for functions.

@alexkimin
Copy link
Copy Markdown

Project Workflow: 4 / 5
Technical Requirement: 4 / 5
Creativity: 3 / 5
Code Quality: 3 / 5
Problem Solving: 3 / 5

Glow

The auto-generation logic of game board, sexy music.
Good progress on self-learning.
Step by step approach based on the time schedule.

Grow

You may use parameters for functions, it will help you to reduce some repetition.

Things to look for

Keep practicing :) - Handling functions, CSS.

Comment thread assets/js/script.js
var $dieButton = $('.dieButton')
var $dieValue = $('.dieValue')
$dieButton.on('click', rollDice)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is "dieButton" a spelling error? Might want to change it to $diceButton

Comment thread assets/js/script.js Outdated
}

function whoWon () {
if ((totalStepsTakenByP1 >= 100)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is there a double (( )) here and in line 82?

Comment thread assets/css/stylesheet.css

.bg {
background-image: url("../images/bg4.jpg");
height: 100%;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

might want to specific that width shall be 100%

Comment thread assets/css/stylesheet.css

.container {
border: 1px solid red;
max-width: 1500px;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this supposed to be max or min?

Comment thread index.html
<div class="wrapper2">
<div class='top-row right-section'>
<img class='gold' src="./assets/images/gold.png" alt="gold image">
<button class="sound" type="button"><img src="./assets/images/sound.png" alt ="auto play"></button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might want to explain how your button works to the user

Comment thread index.html
<div class='top-row right-section'>
<img class='gold' src="./assets/images/gold.png" alt="gold image">
<button class="sound" type="button"><img src="./assets/images/sound.png" alt ="auto play"></button>
<button class="resetButton" type="button"><img src="./assets/images/reset.png" alt ="reset Button"></button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might want to explain how your button works to the user

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.

3 participants