-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
Feature/complete workflow #19
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LVGTM. Just a few comments.
return { | ||
key: c.key, | ||
visible: werewolfService.isInDeck(c.key, currentDeck), | ||
value: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this is not correct, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 1
hardcoded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that's required to be hardcoded (for now). That's the initial amount that a card will have. In order to remove this hardcode number we should have a centralized storage of cards amount (yes, this actually exists in the werewolf brain but the api doesn't return cards amounts).
src/js/pages/Cards.js
Outdated
<label><input type="checkbox" name={c.role} /> {c.role}</label> | ||
</div> | ||
<div class="col-md-6 col-xs-12" key={c.key}> | ||
<label><input type="checkbox" onChange={this.setVisibility} checked={c.visible || false} name={c.key} /> {c.key}</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.visible || false
is not enough c.visible
?
import werewolfService from '../services/werewolf'; | ||
|
||
const DEFAULT_GAME_MODE = 'balanced'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we should create another file with this 'balanced
', 'chaosand
'basic'`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<button onClick={browserHistory.goBack} className="btn btn-default pull-left col-md-2 col-xs-12 btn-space"><i class="fa fa-arrow-left" aria-hidden="true"></i> Back</button> | ||
<button onClick={browserHistory.goBack} className="btn btn-default col-md-2 col-xs-12 btn-space"><i class="fa fa-arrow-left" aria-hidden="true"></i></button> | ||
<button onClick={this.props.startGame.bind(this, "chaos")} className="btn btn-default col-md-4 col-md-offset-1 col-xs-12 btn-space"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</button> | ||
<button onClick={this.props.startGame.bind(this, "balanced")} className="btn btn-default col-md-4 col-md-offset-1 col-xs-12 btn-space"><i class="fa fa-balance-scale" aria-hidden="true"></i> Quick Balanced</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we should create another file with this 'balanced
', 'chaosand
'basic'`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Link to={{pathname:"game", query:{type: "chaos"} }} className="btn btn-default col-md-4 pull-left"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</Link> | ||
<Link to={{pathname:"game", query:{type: "balanced"} }} className="btn btn-default col-md-4 pull-right"><i class="fa fa-balance-scale" aria-hidden="true"></i> Quick Balanced</Link> | ||
<button onClick={this.props.startGame.bind(this, "chaos")} className="btn btn-default col-md-5"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</button> | ||
<button onClick={this.props.startGame.bind(this, "balanced")} className="btn btn-default col-md-5 col-md-offset-2"><i class="fa fa-balance-scale" aria-hidden="true"></i> Quick Balanced</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we should create another file with this 'balanced
', 'chaosand
'basic'`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Link to={{pathname:"game", query:{type: "chaos"} }} className="btn btn-danger pull-right col-md-4 col-xs-12 btn-space"><i class="fa fa-arrows" aria-hidden="true"></i> Start Chaos </Link> | ||
<button onClick={browserHistory.goBack} className="btn btn-default pull-left col-md-2 col-xs-12 btn-space"><i class="fa fa-arrow-left" aria-hidden="true"></i> Back</button> | ||
<button onClick={browserHistory.goBack} className="btn btn-default col-md-2 col-xs-12 btn-space"><i class="fa fa-arrow-left" aria-hidden="true"></i></button> | ||
<button onClick={this.props.startGame.bind(this, "chaos")} className="btn btn-default col-md-4 col-md-offset-1 col-xs-12 btn-space"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we should create another file with this 'balanced
', 'chaosand
'basic'`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</div> | ||
<hr /> | ||
<div class="col-md-12"> | ||
<Link to={{pathname:"game", query:{type: "chaos"} }} className="btn btn-default col-md-4 pull-left"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</Link> | ||
<Link to={{pathname:"game", query:{type: "balanced"} }} className="btn btn-default col-md-4 pull-right"><i class="fa fa-balance-scale" aria-hidden="true"></i> Quick Balanced</Link> | ||
<button onClick={this.props.startGame.bind(this, "chaos")} className="btn btn-default col-md-5"><i class="fa fa-arrows" aria-hidden="true"></i> Quick Chaos</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we should create another file with this 'balanced
', 'chaosand
'basic'`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/js/services/werewolf.js
Outdated
} | ||
|
||
getCards() { | ||
return brain.getAllCards(); | ||
const cards = brain.getCards(); | ||
return Object.keys(cards).map(_ => {return {key: _, value: cards[_]}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a full fan of always having very descriptive names!
No description provided.