Add Register and Sign Up forms #9
Conversation
…c_forms_implementation
| breakfastField, | ||
| onCostChange, | ||
| }: AccomodationFieldGroupProps) => { | ||
| const [groupState, setGroupState] = useState<GroupState>({ |
There was a problem hiding this comment.
It seems like a perfect use case for reducer
app/client/templates/Register.tsx
Outdated
|
|
||
| const [accomodationCostPerGroup, setAccomodationCostPerGroup] = | ||
| React.useState( | ||
| accomodationCheckboxesGroups.map(({ dinner, accomodation, breakfast }) => |
There was a problem hiding this comment.
That looks wrong; why is this a state and not a simple variable?
There was a problem hiding this comment.
Refactored, look comment below.
app/client/templates/Register.tsx
Outdated
| ); | ||
|
|
||
| const [discountPerGroup, setDiscountPerGroup] = React.useState( | ||
| accomodationCheckboxesGroups.map(({ accomodation }) => |
There was a problem hiding this comment.
Refactored, look comment below.
app/client/templates/Register.tsx
Outdated
| ), | ||
| ); | ||
|
|
||
| const generateAccomodationGroupCostCallback = (groupNumber: number) => { |
There was a problem hiding this comment.
That whole logic looks weird. What are we trying to do here?
There was a problem hiding this comment.
We need custom logic for accomodation checkboxes - if you check accomodation for a given day, then breakfast and dinner will be checked automatically, but you can uncheck them later manually. And if you don't check accomodation for a given day, then breakfast and dinner should be disabled.
At the same time we need to calculate total accomodation costs based on all of these checkboxes. And their state is kept inside the <AccomodationFieldGroup/> component, so that's why I created this callback, so that the parent gets updates whenever checkboxes are updated.
I refactored this part by synchronizing state of <AccomodationFieldGroup/> with Reactivated's form state kept inside useForm() hook in the Register template. Let me know what you think about this solution.
| }, | ||
| }); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
I don't like this. If we need to keep it synchronized with Reactivated form state, we must change this logic to use those handlers. Currently, we are doing unnecessary rerender just to keep the state synchronized when we can directly operate on main state.
There was a problem hiding this comment.
Done, it actually made the code much simpler.
I was struggling to figure out how to work with Reactivated's form state and I think this is the way, thanks for this comment :)
This PR contains:
/accounts/signup/form/accounts/register/form/accounts/logoutnow redirects user to home page after logout instead of showing a separate logout page<CenteredContainer/>, so that they behave consistently on different screen sizesTODO next: