Added comments#2
Open
harrydshapiro wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've added some specific comments - you'll see them in "files changed".
Two thoughts that don't really fit anywhere specifically:
What were they interested in seeing from you? Nailing the graphic design? Your state management strategy? The component hierarchy? You have limited time in the interview process, so its important to explicitly ask that question and then take shortcuts on the unimportant stuff so that you can focus on the big stuff.
For example: if they wanted to see how well you can turn a mock/concept into html/css, then you should avoid MaterialUI and make the components yourself (btw - JSX is just fancy HTML and React can import CSS, so don't worry about React being different from html/css). But if they were interested in state management, then MaterialUI is the perfect choice.
Did you consider just using React for your state management (instead of Redux/LocalStorage)? I'd be interested to hear why you think it did/did not fit the task well.
For context - in interview scenarios, I don't think there's usually a reason to use Redux. It introduces a LOT of boilerplate and complexity. Even in production apps sometimes devs don't think the tradeoff is worth it. And localStorage is pretty clunky - we talked about some reasons for that earlier.