Skip to content
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

Edges / Chantelle / Exquisite React #11

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

BASIC-Belic
Copy link

@BASIC-Belic BASIC-Belic commented Dec 13, 2018

Exquisite React

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
How did this project differ from in-class examples? Maybe in wave 3 where we modified and hide a lot of stuff. I tried to use the declarative way of doing this instead of ternary/conditional, where it made sense. Also, most of state was in Game instead of App. App's not doing anything really -- it's just a container for all of its components.
How was this project similar to in-class examples? Similar concepts of nesting, form being the only child component holding on to state.
If you had time to refactor one part of this project, what would you do to make it better? I would make the generation of input fields more dynamic. I almost did in the beginning, but I put the original state in onComponentDidMount, and used that state in a named variable in render function to populate the input attributes. However, when I went into that named variable, I saw that onComponentDidMount was operating asynchronously, and the state was empty the first time, and then loaded, so couldn't get it to work and just abandoned it in favor of finishing this assignment on time. I'll probably try again because I was so close. (Also, just a question: should the App or the Game keep track of the end? I think it makes sense that the Game does, just curious.)

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.

1 participant