-
Notifications
You must be signed in to change notification settings - Fork 6
Pizza flavor question for registration page #228
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
base: main
Are you sure you want to change the base?
Conversation
…etails, removed API route for pizza, started working on adding it to PATCH /applications/personal
Feature looks like it's working, needs reviewing :) |
|
@@ -81,6 +88,32 @@ function ExtraDetailsForm({ application }: { application: Application }) { | |||
if (application.tShirtSize == null) router.push("/dashboard/education") | |||
} | |||
|
|||
const [snackChoice, setSnackChoice] = useState<string>( | |||
!application.pizzaFlavors?.includes("alternative") && !application.pizzaFlavors?.includes("nothing") |
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.
Code like this is why the choice of pizza/alternative/nothing should be separate to pizza flavour selection
: application.pizzaFlavors[0], | ||
) | ||
|
||
function handleSnackChoice(value: string): void { |
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.
Do we use a function like this anywhere else in our forms? I think state is mostly handled for us?
<FormItem> | ||
<FormLabel>Midnight Snack (on us)</FormLabel> | ||
<FormDescription> | ||
We normally offer pizza as a midnight snack. If you want something else, select "alternative" and we |
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.
This should be rephrased to communicate that people cannot ask for an alternative just because they fancy it
eg 'if pizza is not suitable for you'
</SelectContent> | ||
</Select> | ||
|
||
{snackChoice === "pizza" && ( |
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.
aha so it is a separate question, to the user, but the user application format wasn't updated to include a midnightSnack
member and that's where the weirdness is coming from
|
||
{snackChoice === "pizza" && ( | ||
<> | ||
<FormDescription>Please choose the flavours of pizza you would be okay to have</FormDescription> |
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.
Phrasing is off here but I'm not sure how to fix it
@@ -42,7 +42,7 @@ function UserAttribute({ children, className, ...props }: React.HTMLAttributes<H | |||
} | |||
|
|||
export default async function ProfilePage(props: { params: Promise<{ userId: string }> }) { | |||
const params = React.use(props.params); | |||
const params = React.use(props.params) |
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.
This is an unrelated change but should certainly be committed to main
- if I were more strict I'd ask you to put it in a separate PR
Should close #201 when finished