Adam Reid - [LambdaMUD-Client]#112
Conversation
TheDeterminator
left a comment
TheDeterminator
left a comment
There was a problem hiding this comment.
Adam, everything looks great! Please refer to my sprint review comments for a better summary of my overall impression of this project.
| @@ -0,0 +1,140 @@ | |||
| import axios from 'axios'; | |||
There was a problem hiding this comment.
Nice! I know you showed us your logs but I forgot that you used Redux in here. Good job!
| // if(this.state.command === 'n' || this.state.command === 's' || this.state.command === 'e' || this.state.command === 'w'){ | ||
| if(this.state.command && this.state.command[0] !== '/'){ | ||
| let direction = ''; | ||
| switch(this.state.command){ |
There was a problem hiding this comment.
This is good but looking at the syntax it makes me wonder if you couldn't have just set the direction variable directly from the command attribute instead of using a switch statement to set it (and then having error handling if the user inputs an invalid command). Maybe I'm not seeing the full picture of this method though.
There was a problem hiding this comment.
I think this was more of an exercise in utilizing switch/case statements than anything else, as I haven't built them as often as I've built nested if statements, which can become jumbled and messy if they go on too long. The idea behind this input sanitization was to ensure that the backend receives the proper value, which is just a single character direction, instead of the full direction string (e.g. 'n' vs 'north'), while still giving users some freedom as to how they want to move directions.
| password2: this.state.password2 | ||
| } | ||
| this.props.register(newUser); | ||
| } |
There was a problem hiding this comment.
Why doesn't this load the game screen after registration? Seems like that might be desirable beahviour, like you have for the login.
There was a problem hiding this comment.
I had cut the registration redirect while attempting to implement the redirect in another place (actions/reducers), so that the pusher registration could propagate properly on the redirect. I was unable to find a solution in time for the demos, so it just never got put back in. It's back for now, though there's a package (react-router-redux) that may offer some insight as to how to implement redux redirects that I'm looking into.
2 participants
No description provided.