-
Notifications
You must be signed in to change notification settings - Fork 14
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
Draft a simple React/Typescript exercise #7
base: master
Are you sure you want to change the base?
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.
I like this challenge, not overwhelming if you know what you're doing 👍 Left few comments.
|
||
## Requirements | ||
|
||
1. Use Typescript |
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.
And React 18? Should we specify the TS version too?
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.
Or hmm, did you mean that alternative 1) can be implemented in something else than React?
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.
React 18 because server components are only available in it. That said, we should specify React as a requirement for the first one as well.
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.
Maybe the first alternative is missing some oomph or interactivity? Should there be at least some state that's being handled?
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.
Good point and I agree, there should be at least some state management included in this challenge 🤔
What's the most purposeful way then, state changes by reacting to external data changes (polling graphql? etc) or by having user interaction? Including both may bloat the scope of the challenge too much.
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.
How about adding simple "Subscribe to newsletter" form. That should handle quite many use cases for state, handling controlled inputs, loading states, success and error? We could setup up mock api that randomly sends back either 200 OK or some server error?
|
||
1. Use Typescript | ||
2. Document setting up the development environment with a README | ||
3. The website should be responsive |
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.
When talking about requirements, let's be imperative: "Make website responsive."
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.
Good point.
A bit frontend-y, perhaps, but do you have some suggestions?