Skip to content

NSCP review fixes: deeper pass on top of recent surgical fixes (#23) - #2

Open
EfeosasereOkoro-gov wants to merge 1 commit into
mainfrom
efe/nscp-deeper-fixes
Open

NSCP review fixes: deeper pass on top of recent surgical fixes (#23)#2
EfeosasereOkoro-gov wants to merge 1 commit into
mainfrom
efe/nscp-deeper-fixes

Conversation

@EfeosasereOkoro-gov

Copy link
Copy Markdown
Collaborator

Summary

Comprehensive review pass on the NSCP Camper Registration form addressing the 22 items in govtech-bb/projects#23 that werent covered by the recent surgical fixes on main.

Two reviewers (manual + Claude) raised issues across Critical / Significant / Minor. This PR layers the structural and framework-level changes on top of the inline copy/validation fixes already merged.

Live preview (after merge):https://govtech-bb.github.io/newforms/Prototypes/nscp-camper-registration.html

What is in this PR

Framework (assets/govbb-framework.js)

  • render(preserveScroll) -- only scrolls to top on real next/back/nav. Conditional re-renders preserve scroll + restore keyboard focus to the just-clicked control.
  • Radio inputs use change event (was input) and reliably restore checked state on every render.
  • New _clearFieldError(el) -- clears inline error message, aria-invalid, and the matching <li> in the error summary as soon as the user fixes a field. Handles radio groups by name.
  • _hidePreviousOnFirstStep now actually wired up in render().

These framework improvements benefit every prototype using govbb-framework.js, not just NSCP.

NSCP form (Prototypes/nscp-camper-registration.html)

  • Flow restructure: "Choose a camp" was one long page. Split into three steps -- Choose a camp (parish/venue/topics) -> Activities and first aid -> Photos and videos.
  • Emergency-contact step always visible: removed the getFlow filter that dropped the step when guardian = emergency contact. When that's the case, the page renders a confirmation-only view ("You said you are the emergency contact... change") with Continue. Total form steps stable at 11 -- fixes the 9->8 step counter jump.
  • Name split: Camper / Guardian / Emergency contact each get First name / Other names (optional) / Surname instead of one Full name field. (Mirrors what was done on the centenarian form.)
  • First-aid consent is now an explicit required checkbox on the new Activities page (was passive notice copy).
  • DOB validation flags all three fields (Day/Month/Year) when any are missing -- error summary link still focuses Day.
  • Medication Authorisation Form: download link added above the upload widget.
  • Confirmation page: email confirmation mention, "Print or save this page" button, "Register another child" link.
  • Sex / Gender consistency: review page reads "Sex".
  • Checkbox alignment: all 14 checkboxes (curriculum + first-aid + conduct) top-aligned with consistent spacing.
  • Topics hint: clarifies at-least-one requirement.

Relationship to the recently-merged fixes (#1)

The commit on main (570773b) made small surgical fixes to 6 items in the NSCP HTML. Two of those overlap with this PR:

  • Dietary conditional on meals=Yes -- Gavin used inline conditional logic; this PR uses the framework's data-show-when-name pattern (so the same conditional reveal style as every other Yes/No in the form). Behaviour is the same.
  • "Error:" title prefix cleared on resolution -- Gavin cleared it on form pages; this PR clears it from the framework's _clearFieldError so it works everywhere automatically.

Both overlaps should be reviewed; if the framework-level versions look cleaner, the inline ones can come out in a follow-up. No functional regression either way.

Test plan

  • Click any Yes/No radio on the Medical & health step -- page does NOT scroll to top, focus stays on the radio
  • Continue without selecting illness -- error shows; click "Yes" -- error clears immediately
  • Leave DOB blank, click Continue -- Day / Month / Year all show red
  • Step 1 has First name / Other names / Surname inputs (not "Full name")
  • On Meals, select Yes -> dietary appears; No -> dietary hides; navigating forward + back preserves the selection
  • Choose "I am the emergency contact: Yes" -> still see an emergency-contact step (confirmation-only view), counter stays at "Step X of 11"
  • Activities page has an explicit first-aid consent checkbox; Continue without ticking shows an error
  • Confirmation page shows email mention, Print button, and Register another child link
  • Other prototypes (terms-leave, textbook-grant, exam-fee-refund) still work -- the framework changes shouldn't have broken them

…cture
Addresses 22 review items from #23 across both reviewers.
Framework (govbb-framework.js):
- render() preserves scroll + focus when called for conditional re-renders;
only navigates-to-top on real next/back/nav
- Radio inputs now use 'change' event and restore checked state on re-render
- New _clearFieldError() clears inline errors, aria-invalid, and matching
summary list items as user fixes each field
- _hidePreviousOnFirstStep() wired up in render()
NSCP form (nscp-camper-registration.html):
- Flow restructured: camp page split into Camp / Activities / Photos
- Emergency-contact step always visible (step counter stable at 11)
- Name fields split into First / Other (optional) / Surname for camper,
guardian, and emergency contact
- Meals: conditional dietary reveal when Yes; data-trigger-render added
- DOB validation flags all 3 fields (Day/Month/Year), not just Day
- First-aid consent now an explicit required checkbox on activities page
- Medication Authorisation Form download link added
- Confirmation page: email mention, print/save button, register another child
- Check page: 'Sex' label, new activities + photos sections
- Topics-of-interest hint clarifies at-least-one requirement
- All 14 checkboxes now top-aligned with consistent spacing
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@amazon-inspector-n-virginia

Copy link
Copy Markdown

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@amazon-inspector-n-virginia

Copy link
Copy Markdown

✅ I finished the code review, and didn't find any security or code quality issues.

Sign up for freeto 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.

1 participant

@EfeosasereOkoro-gov