Skip to content

Feature/signup page#44

Open
Richman-Tan wants to merge 24 commits intomainfrom
feature/signup-page
Open

Feature/signup page#44
Richman-Tan wants to merge 24 commits intomainfrom
feature/signup-page

Conversation

@Richman-Tan
Copy link
Copy Markdown

What does this PR do?

This PR introduces styling improvements to the sign-up page, restructuring it into clearly defined sections to improve readability and user experience. It also adds additional input fields required for Stripe integration, enabling future payment setup and onboarding flows.


Type of change

  • Feature
  • Bug fix
  • Chore / config
  • Hotfix

Checklist

  • My branch follows the naming convention (feature/, fix/, chore/, hotfix/)
  • My commit messages follow the conventional commits format
  • CI checks pass

deb-coder-man and others added 2 commits April 28, 2026 08:10
…rs and send payment link to the frontend, created a basic sign up form and a success screen for testing purposes
Copilot AI review requested due to automatic review settings April 28, 2026 11:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a multi-step sign-up flow in the web app and wires it into a new CMS-backed Stripe Checkout + webhook flow, along with new member profile fields needed for onboarding/payment context.

Changes:

  • Adds a redesigned, multi-step /signup page (client-side) plus a /signup/success landing page.
  • Adds a web /api/checkout route that proxies checkout creation requests to the CMS.
  • Adds CMS Stripe routes (/stripe/checkout, /stripe/webhook) and extends the members collection/types with additional onboarding fields.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
web/src/app/signup/page.tsx New multi-step signup UI that collects onboarding fields and initiates Stripe checkout.
web/src/app/signup/success/page.tsx New success page that shows a message based on presence of session_id.
web/src/app/api/checkout/route.ts Server-side proxy from web app to CMS Stripe checkout endpoint.
cms/src/app/stripe/checkout/route.ts Creates a pending member and a Stripe customer/session; returns checkout URL.
cms/src/app/stripe/webhook/route.ts Verifies Stripe signature and activates member on successful payment.
cms/src/collections/Members.ts Adds new member fields and introduces create access configuration.
cms/src/payload-types.ts Updates generated Payload types for the new member fields.
cms/src/app/(payload)/admin/importMap.js Import map formatting change.
cms/package.json Adds Stripe dependency (and reorders dependencies).
cms/pnpm-lock.yaml Lockfile updates for Stripe and related dependency metadata changes.
cms/.env.example Documents required Stripe-related environment variables.
Files not reviewed (1)
  • cms/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +71 to +75
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<input
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputField renders a <label> but it isn’t associated with the <input> via htmlFor/id, which reduces accessibility (screen readers / label click-to-focus). Consider generating/passing an id and wiring label to input (and optionally adding aria-invalid when error is set).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what copilot is mentioning here is that if a user who is blind or deaf wants to use voice controls e.g., they say "click First Name" the acessibility control wouldn't work properly?

Fix should be useId + htmlFor + passing required/aria-invalid through to the input. I haven't dug into accessibility much beyond this so worth double-checking the exact attributes, but the basic shape of the fix is solid.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +104 to +109
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<select
value={value}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SelectField has the same accessibility issue as InputField: the <label> isn’t linked to the <select> via htmlFor/id. Hooking these up will improve keyboard/screen-reader behavior and make clicking the label focus the control.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as my reply on the InputField comment above, useId + htmlFor on the label, plus aria-required on the select. The accessibility impact is identical (screen reader users don't hear the field label, voice-control users can't say "click Gender" to focus it, mobile users lose the bigger label tap target).

Comment on lines +3 to +7
interface Props {
searchParams: Promise<{ session_id?: string }>
}

export default async function SignupSuccessPage({ searchParams }: Props) {
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

searchParams is typed as a Promise and then awaited, but Next.js passes searchParams as a plain object. Also, using only the presence of session_id to show “payment was successful” is easy to spoof via the URL. Consider typing searchParams as an object and (if you want a strong success message) verifying the session/payment status server-side before rendering.

Copilot uses AI. Check for mistakes.
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/webhook/route.ts
Comment thread cms/src/collections/Members.ts Outdated
Comment thread cms/src/app/stripe/webhook/route.ts Outdated
Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +353 to +355
function handleNext() {
if (step < TOTAL_STEPS) setStep((s) => s + 1)
}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleNext advances steps without validating the current step’s required fields, so users can click through to payment with missing data (despite the UI marking fields as required). Consider validating per-step on Next (and blocking progression / surfacing errors) to keep the submitted payload consistent.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as my reply on the validate() comment further down

Copilot AI review requested due to automatic review settings April 29, 2026 01:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.

Files not reviewed (1)
  • cms/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cms/src/app/stripe/checkout/route.ts Outdated
Comment thread cms/.env.example
Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +8 to +22
type FormData = {
firstName: string
lastName: string
email: string
phone: string
password: string
confirmPassword: string
upi: string
studentId: string
areaOfStudy: string
yearOfUniversity: string
gender: string
ethnicity: string
returningMember: string
}
Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +73 to +84
<div className="flex flex-col gap-1">
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<input
type={type}
placeholder={placeholder}
value={value}
onChange={(e) => onChange(e.target.value)}
className="w-full rounded-lg px-3 py-2 text-sm outline-none border border-transparent focus:border-ssa-red bg-white placeholder:text-gray-400"
/>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same root issue as the InputField/SelectField label comments so fixing those will pick up most of this. The extra bit Copilot's calling out here is also passing the native required attribute through to the <input>/<select> itself, not just rendering the asterisk.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +382 to +398
const validate = () => {
const errors: Record<string, string> = {}
if (!formData.firstName.trim()) errors.firstName = 'First name is required'
if (!formData.lastName.trim()) errors.lastName = 'Last name is required'
if (
!formData.email.trim() ||
!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(formData.email)
) {
errors.email = 'Valid email is required'
}
if (!formData.phone.trim()) errors.phone = 'Phone number is required'
if (formData.password.length < 8)
errors.password = 'Password must be at least 8 characters'
if (formData.password !== formData.confirmPassword)
errors.confirmPassword = 'Passwords do not match'
return errors
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with what Copilot said here, right now user can just click next through all the pages without having to fill in the necessary fields.

What copilot suggest should be able to fix the bug, just by extending validate() to cover steps 2/3 and run validation in handleNext() so users see errors as they go rather than getting redirected from Step 4 with no explanation.

Comment thread cms/src/app/stripe/webhook/route.ts Outdated
Comment thread cms/src/app/stripe/webhook/route.ts Outdated
}

const { name, email, phone, encryptedPassword } = session.metadata ?? {}
const stripeCustomerId = session.customer as string | null
Comment thread cms/src/app/stripe/checkout/route.ts

export const importMap = {
'@payloadcms/next/rsc#CollectionCards': CollectionCards_f9c02e79a4aed9a3924487c0cd4cafb1,
"@payloadcms/next/rsc#CollectionCards": CollectionCards_f9c02e79a4aed9a3924487c0cd4cafb1
Comment thread cms/src/app/stripe/checkout/route.ts
Copy link
Copy Markdown
Collaborator

@joengy joengy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall theres only a few small accessibility changes needed, most of the comments were about the changes you pulled from Dave's branch so you don't have to worry about those.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +71 to +75
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<input
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what copilot is mentioning here is that if a user who is blind or deaf wants to use voice controls e.g., they say "click First Name" the acessibility control wouldn't work properly?

Fix should be useId + htmlFor + passing required/aria-invalid through to the input. I haven't dug into accessibility much beyond this so worth double-checking the exact attributes, but the basic shape of the fix is solid.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +382 to +398
const validate = () => {
const errors: Record<string, string> = {}
if (!formData.firstName.trim()) errors.firstName = 'First name is required'
if (!formData.lastName.trim()) errors.lastName = 'Last name is required'
if (
!formData.email.trim() ||
!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(formData.email)
) {
errors.email = 'Valid email is required'
}
if (!formData.phone.trim()) errors.phone = 'Phone number is required'
if (formData.password.length < 8)
errors.password = 'Password must be at least 8 characters'
if (formData.password !== formData.confirmPassword)
errors.confirmPassword = 'Passwords do not match'
return errors
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with what Copilot said here, right now user can just click next through all the pages without having to fill in the necessary fields.

What copilot suggest should be able to fix the bug, just by extending validate() to cover steps 2/3 and run validation in handleNext() so users see errors as they go rather than getting redirected from Step 4 with no explanation.

Comment thread web/src/app/signup/page.tsx Outdated
placeholder={placeholder}
value={value}
onChange={(e) => onChange(e.target.value)}
className="w-full rounded-lg px-3 py-2 text-sm outline-none border border-transparent focus:border-ssa-red bg-white placeholder:text-gray-400"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputField <input> className at L83, and SelectField <select> style at L115.

The text in the input fields renders in a very light grey, almost identical to the placeholder colour, so once you start typing your input is barely readable against the white background. I think cause is the <input> having no explicit text colour set and only bg-white and placeholder:text-gray-400.

Image

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +8 to +12
type FormData = {
firstName: string
lastName: string
email: string
phone: string
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore this comment as we're going to scrap the integrate form stuff after the execs mentioned how important it is to have a system where they can create events and manage waitlist etc in a non-spread sheet format.

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +104 to +109
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<select
value={value}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as my reply on the InputField comment above, useId + htmlFor on the label, plus aria-required on the select. The accessibility impact is identical (screen reader users don't hear the field label, voice-control users can't say "click Gender" to focus it, mobile users lose the bigger label tap target).

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +353 to +355
function handleNext() {
if (step < TOTAL_STEPS) setStep((s) => s + 1)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as my reply on the validate() comment further down

Comment thread web/src/app/signup/page.tsx Outdated
Comment on lines +73 to +84
<div className="flex flex-col gap-1">
<label className="text-sm font-medium text-ssa-black">
{label}
{required && <span className="text-ssa-red ml-0.5">*</span>}
</label>
<input
type={type}
placeholder={placeholder}
value={value}
onChange={(e) => onChange(e.target.value)}
className="w-full rounded-lg px-3 py-2 text-sm outline-none border border-transparent focus:border-ssa-red bg-white placeholder:text-gray-400"
/>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same root issue as the InputField/SelectField label comments so fixing those will pick up most of this. The extra bit Copilot's calling out here is also passing the native required attribute through to the <input>/<select> itself, not just rendering the asterisk.

Richman-Tan and others added 2 commits May 1, 2026 15:44
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 1, 2026 04:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • cms/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread web/src/app/api/checkout/route.ts Outdated
Comment thread cms/.env.example Outdated
Comment thread web/src/app/signup/page.tsx Outdated
Comment thread web/src/app/signup/success/page.tsx Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 04:09
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Richman-Tan and others added 3 commits May 1, 2026 16:09
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • cms/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web/src/app/signup/success/page.tsx
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts Outdated
Comment thread web/src/app/signup/page.tsx Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 04:16
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ipe button

Agent-Logs-Url: https://github.com/UoaWDCC/ssa/sessions/c67c821e-aeb4-4238-a4bb-1c8d46139541

Co-authored-by: Richman-Tan <160602954+Richman-Tan@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented May 1, 2026

@copilot apply changes based on the comments in this thread

Applied all four changes from review 4209899554 in commit dc5ca6c:

  1. Server-side validation — extended the validation check to include all required fields: UPI, studentId, areaOfStudy, yearOfUniversity, gender, ethnicity, and returningMember (not just name/email/password/phone).
  2. "Powered by Stripe" button — changed from a no-action <button> to a <p> element since it's non-interactive text.
  3. overrideAccess: true — was already present from a previous commit.
  4. Update pending member on retry — when an existing pending member is found, the route now updates their details (name, phone, UPI, studentId, areaOfStudy, yearOfUniversity, gender, ethnicity, returningMember) with the latest submitted values. Password is intentionally excluded from the update to avoid unnecessary re-hashing of an already-stored value.

Comment thread web/src/app/signup/page.tsx Outdated
style={{ color: value ? '#0a0805' : '#9ca3af' }}
>
<option value="" disabled hidden>
{placeholder ?? 'DROP DOWN'}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is based off the Figma but could we change all instances of 'DROP DOWN' to 'Select an option'.

Also will need to add an up/down arrow within the element to indicate visually that it's a dropdown (I've asked design to update the Figma accordingly to these changes)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course Ms Gandhi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in an earlier commit — SelectField now defaults the placeholder to 'Select an option' and includes a chevron SVG arrow indicator to the right of the dropdown.

Comment thread web/src/app/signup/page.tsx Outdated
)
}

function Step1({
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing 'step1' 'step2' etc to 'contactStep' 'uniInfoStep' would be more readable

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course Ms Gandhi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in an earlier commit — the step components are now named ContactStep, UniInfoStep, and AdditionalInfoStep.

Copilot AI review requested due to automatic review settings May 1, 2026 04:54
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Files not reviewed (1)
  • cms/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cms/src/app/stripe/webhook/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread web/src/app/api/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment on lines +24 to +69
let event: Stripe.Event
try {
event = stripe.webhooks.constructEvent(rawBody, signature, webhookSecret)
} catch (err: unknown) {
const message = err instanceof Error ? err.message : 'Webhook signature verification failed'
return Response.json({ error: message }, { status: 400 })
}

if (event.type === 'checkout.session.completed') {
const session = event.data.object as Stripe.Checkout.Session

if (session.payment_status !== 'paid') {
return Response.json({ received: true })
}

const { memberId: rawMemberId } = session.metadata ?? {}
const stripeCustomerId =
typeof session.customer === 'string' ? session.customer : session.customer?.id ?? null

const memberId = Number(rawMemberId)
if (!Number.isFinite(memberId)) {
console.error('Stripe webhook: invalid or missing memberId in session metadata', {
sessionId: session.id,
rawMemberId,
})
return Response.json({ received: true })
}

const payload = await getPayload({ config: configPromise })

try {
await payload.update({
collection: 'members',
id: memberId,
data: {
status: 'active',
...(stripeCustomerId ? { stripeCustomerId } : {}),
},
})
} catch (err: unknown) {
const message = err instanceof Error ? err.message : 'Failed to update member'
console.error('Stripe webhook: failed to update member', { memberId, error: message })
// Return 500 so Stripe retries with exponential backoff
return Response.json({ error: message }, { status: 500 })
}
}
Copy link
Copy Markdown
Collaborator

@oorjagandhi oorjagandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks really good thank you!
Could you please add the hero component on top and change the props accordingly to match Figma.

Also for readability could you extract out the frontend pages into components 🙏

Copy link
Copy Markdown
Collaborator

@oorjagandhi oorjagandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes!! Just a few minor things:

  • remove all inline styling and replace with tailwindCSS where possible. I commented on instances I spotted at a glance but can you make sure its fixed everywhere
  • for the components folder, could you move the components: InputField, SelectField, CardSection, ProgressBar, and PaymentStep into "web/src/components" so they can be reused in other parts of code

return (
<div
className="w-full h-3 rounded-full overflow-hidden"
style={{ backgroundColor: '#ffb3bf' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this colour be defined in global.css / select an existing colour defined in global.css that looks similar

>
<div
className="h-full rounded-full transition-all duration-300"
style={{ width: `${progress}%`, backgroundColor: '#f85b76' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ssa-red

required={required}
aria-invalid={!!error}
className="w-full rounded-lg px-3 py-2 pr-8 text-sm outline-none border border-transparent focus:border-ssa-red bg-white appearance-none"
style={{ color: value ? '#0a0805' : '#9ca3af' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ssa-black, can u define other colour in global.css

<button
onClick={handleNext}
className="px-6 py-2 rounded-full text-sm font-medium text-white"
style={{ backgroundColor: '#f85b76' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ssa-red

style={{ backgroundColor: '#ffe6b6' }}
>
<div>
<h2 className="font-[family-name:var(--font-averia)] font-bold text-xl text-ssa-black">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h2 className="font-[family-name:var(--font-averia)] font-bold text-xl text-ssa-black">
<h2 className="font-averia font-bold text-xl text-ssa-black">

}) {
return (
<div
className="rounded-2xl p-6 flex flex-col gap-5"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
className="rounded-2xl p-6 flex flex-col gap-5"
className="bg-ssa-yellow rounded-2xl p-6 flex flex-col gap-5"

return (
<div
className="rounded-2xl p-6 flex flex-col gap-5"
style={{ backgroundColor: '#ffe6b6' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove inline css, use tailwindCSS instead


return (
<div
className="min-h-screen flex flex-col"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
className="min-h-screen flex flex-col"
className="bg-ssa-yellow-light min-h-screen flex flex-col"

return (
<div
className="min-h-screen flex flex-col"
style={{ backgroundColor: '#fff7e9' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove inline css, use tailwindCSS instead

Copilot AI review requested due to automatic review settings May 2, 2026 09:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Comment thread cms/src/app/stripe/checkout/route.ts
Copy link
Copy Markdown
Collaborator

@oorjagandhi oorjagandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one rename request :)

Comment thread web/src/app/globals.css Outdated
--color-foreground: var(--foreground);
--color-ssa-red: #f85b76;
--color-ssa-red-light: #ff879c;
--color-ssa-red-pale: #ffb3bf;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could u rename pale to lighter for more clarity

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — globals.css now uses --color-ssa-yellow-light (not pale).

Comment thread web/src/components/PaymentStep.tsx Outdated
<button
onClick={onPay}
disabled={isLoading}
className="w-full py-2.5 rounded-lg bg-[#635bff] text-white text-sm font-medium disabled:opacity-60"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick / consistency: This one is intentionally Stripe's brand purple, so calling it --color-stripe-purple (or similar) keeps the Tailwind class self-documenting (bg-stripe-purple).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — the colour is defined as --color-stripe-purple: #635bff in globals.css, so the Tailwind class bg-stripe-purple is self-documenting.

const [error, setError] = useState<string | null>(null)
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({})

function handleChange(field: keyof FormData, value: string) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug / UX: Once a field error is shown, it stays on screen until the user clicks Next or Pay again, so typing/correcting the field doesn't clear it. Standard pattern is to clear that field's error inside handleChange so the user gets immediate feedback that they've fixed it.

Most signup forms I've seen do clear-on-type + validate-on-blur: errors clear as you type, then re-check when you tab away. But that's a bigger change. For this PR, I woudl probably just go with clear-on-type.

Image

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — handleChange in SignupForm.tsx clears the field's error immediately when the user types, so the error disappears as soon as they start correcting it.

errors.email = 'Valid email is required'
}
if (!formData.phone.trim()) errors.phone = 'Phone number is required'
if (formData.password.length < 8)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick / security: Password rule is only length < 8, so 'aaaaaaaa' passes. Worth strengthening to require at least one number and one letter (or a mix of upper/lower/symbol).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — password validation now requires at least 8 characters, at least one letter (/[a-zA-Z]/), and at least one number (/[0-9]/).

onChange={(e) => onChange(e.target.value)}
required={required}
aria-invalid={!!error}
className="w-full rounded-lg px-3 py-2 text-sm text-gray-900 outline-none border border-transparent focus:border-ssa-red bg-white placeholder:text-gray-400"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX/nitpick: No autoComplete or name is forwarded, so password managers and browser autofill can't help here. Could you accept autoComplete and name props, and pass them through? (e.g. autoComplete="given-name", "family-name", "email", "tel", "new-password" for the relevant fields).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — InputField now accepts and forwards autoComplete and name props. All fields in ContactStep pass appropriate values (e.g. autoComplete="given-name", "email", "tel", "new-password").

…, update styles for payment and progress components
Copilot AI review requested due to automatic review settings May 3, 2026 04:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +83 to +102
if (existing.docs.length > 0) {
memberId = existing.docs[0].id
// Update the existing pending member with the latest submitted details
// so a retry after correcting input always uses the most recent data.
await payload.update({
collection: 'members',
id: memberId,
overrideAccess: true,
data: {
name,
phone,
upi,
studentId,
areaOfStudy,
yearOfUniversity,
gender,
ethnicity,
returningMember,
},
})
Comment on lines 137 to 176
let customerId: string
try {
const customer = await stripe.customers.create({ email, name })
customerId = customer.id
} catch (err: unknown) {
if (memberCreatedHere) {
await payload.delete({ collection: 'members', id: memberId }).catch(() => {})
}
const message = err instanceof Error ? err.message : 'Failed to create Stripe customer'
return Response.json({ error: message }, { status: 502 })
}

const encryptedPassword = encryptPassword(password, encryptionKey)

try {
const session = await stripe.checkout.sessions.create({
mode: 'payment',
customer: customerId,
line_items: [{ price: priceId, quantity: 1 }],
success_url: `${webUrl}/signup/success?session_id={CHECKOUT_SESSION_ID}`,
cancel_url: `${webUrl}/signup?cancelled=true`,
metadata: { name, email, phone, encryptedPassword },
metadata: { memberId: String(memberId) },
})

if (!session.url) {
if (memberCreatedHere) {
await payload.delete({ collection: 'members', id: memberId }).catch(() => {})
}
return Response.json(
{ error: 'Stripe did not provide a checkout URL for the created session' },
{ status: 502 },
)
}

return Response.json({ checkoutUrl: session.url })
} catch (err: unknown) {
if (memberCreatedHere) {
await payload.delete({ collection: 'members', id: memberId }).catch(() => {})
}
const message = err instanceof Error ? err.message : 'Failed to create Stripe checkout session'
return Response.json({ error: message }, { status: 502 })
}
Comment thread web/src/components/ProgressBar.tsx Outdated
Comment on lines +10 to +15
<div className="w-full h-3 rounded-full overflow-hidden bg-ssa-red-lighter">
<div
className="h-full rounded-full transition-all duration-300 bg-ssa-red"
style={{ width: `${progress}%` }}
/>
</div>
Comment thread web/src/components/InputField.tsx Outdated
Comment on lines +33 to +45
<input
id={id}
name={name}
type={type}
placeholder={placeholder}
value={value}
onChange={(e) => onChange(e.target.value)}
required={required}
aria-invalid={!!error}
autoComplete={autoComplete}
className="w-full rounded-lg px-3 py-2 text-sm text-gray-900 outline-none border border-transparent focus:border-ssa-red bg-white placeholder:text-gray-400"
/>
{error && <p className="text-xs text-red-600">{error}</p>}
Comment on lines +30 to +65
<select
id={id}
value={value}
onChange={(e) => onChange(e.target.value)}
required={required}
aria-invalid={!!error}
className={`w-full rounded-lg px-3 py-2 pr-8 text-sm outline-none border border-transparent focus:border-ssa-red bg-white appearance-none ${value ? 'text-ssa-black' : 'text-gray-400'}`}
>
<option value="" disabled hidden>
{placeholder ?? 'Select an option'}
</option>
{options.map((o) => (
<option key={o.value} value={o.value}>
{o.label}
</option>
))}
</select>
<svg
className="pointer-events-none absolute right-2.5 top-1/2 -translate-y-1/2 text-gray-400"
width="16"
height="16"
viewBox="0 0 16 16"
fill="none"
aria-hidden="true"
>
<path
d="M4 6l4 4 4-4"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
</svg>
</div>
{error && <p className="text-xs text-red-600">{error}</p>}
</div>
Comment thread web/src/components/PaymentStep.tsx Outdated
<CardSection title="Payment">
<div className="flex flex-col gap-3">
<p className="text-sm text-ssa-black">
$6 is required to be a SSA member, the fee includes:
Comment thread web/src/app/api/checkout/route.ts Outdated
try {
data = JSON.parse(cmsBody)
} catch {
data = { error: cmsBody }
@Richman-Tan
Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

…ing, Stripe customer reuse

Agent-Logs-Url: https://github.com/UoaWDCC/ssa/sessions/9b2537ef-3094-477c-b935-7d1ae87f676e

Co-authored-by: Richman-Tan <160602954+Richman-Tan@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented May 3, 2026

@copilot apply changes based on the comments in this thread

Applied all changes from review 4215882932 in commit 3db8350:

  • ProgressBar — added role="progressbar" + aria-valuemin/max/now + aria-label
  • InputField / SelectField — added aria-describedby on the control pointing to the error <p> (which now has a unique id), so screen readers announce the error alongside the field
  • PaymentStep — "a SSA member" → "an SSA member"
  • api/checkout — CMS non-JSON error bodies are now logged server-side via console.error and replaced with a generic "Checkout service error" message to avoid leaking internals
  • CMS checkout route (two fixes):
    • Password is now included in the pending member update on retry
    • stripeCustomerId is read from the pending member and reused if present (no duplicate Stripe customer per retry); when a new customer is created its ID is saved back to the member immediately; on new-member rollback the orphaned Stripe customer is also deleted (best-effort)

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.

6 participants