From ae4bcef4c9adf77745e5403075e94244c820d5da Mon Sep 17 00:00:00 2001 From: Richie Date: Mon, 30 Mar 2026 08:22:06 +1100 Subject: [PATCH] Quality pass: fix P0 audit findings across FilterPanel, ArrangementDialog, steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FilterPanel (4 P0 + 2 P1 fixes): - Add forwardRef (project convention) - Use React.useId() for unique popover/heading IDs (was static) - Change aria-describedby to aria-controls (correct ARIA pattern) - Add role="dialog" + aria-labelledby on Popover paper - Popover header now uses label prop (was hardcoded "Filters") - Clear all font size uses theme.typography.caption (was hardcoded) - Badge uses aria-hidden + visually-hidden text (cleaner SR output) - Add maxHeight + overflow scroll to body, aria-label on Done button ArrangementDialog (3 P0 + 1 P1 fixes): - Add forwardRef - Focus management: titleRef focused on step change via useEffect - Add aria-live region announcing step transitions to screen readers - Fix borderRadius from 3 to 2 (theme convention) Sticky header padding (visual fix): - ProvidersStep + VenueStep: mx/px now responsive { xs: -2/2, md: -3/3 } matching the panel's px: { xs: 2, md: 3 } — fixes mobile misalignment CoffinDetailsStep: - Wrap CTA area in form element with onSubmit + aria-busy Co-Authored-By: Claude Opus 4.6 (1M context) --- .../molecules/FilterPanel/FilterPanel.tsx | 250 +++--- .../ArrangementDialog/ArrangementDialog.tsx | 711 +++++++++--------- .../CoffinDetailsStep/CoffinDetailsStep.tsx | 11 +- .../pages/ProvidersStep/ProvidersStep.tsx | 4 +- src/components/pages/VenueStep/VenueStep.tsx | 4 +- 5 files changed, 516 insertions(+), 464 deletions(-) diff --git a/src/components/molecules/FilterPanel/FilterPanel.tsx b/src/components/molecules/FilterPanel/FilterPanel.tsx index 9c18cae..704cf2a 100644 --- a/src/components/molecules/FilterPanel/FilterPanel.tsx +++ b/src/components/molecules/FilterPanel/FilterPanel.tsx @@ -39,128 +39,146 @@ export interface FilterPanelProps { * D-C: Popover for desktop MVP. Mobile Drawer variant planned for later. * * Used in ProvidersStep, VenueStep, and CoffinsStep. - * - * Usage: - * ```tsx - * - * - * - * - * ``` */ -export const FilterPanel: React.FC = ({ - label = 'Filters', - activeCount = 0, - children, - onClear, - minWidth = 280, - sx, -}) => { - const [anchorEl, setAnchorEl] = React.useState(null); - const open = Boolean(anchorEl); - const popoverId = open ? 'filter-panel-popover' : undefined; +export const FilterPanel = React.forwardRef( + ({ label = 'Filters', activeCount = 0, children, onClear, minWidth = 280, sx }, ref) => { + const [anchorEl, setAnchorEl] = React.useState(null); + const open = Boolean(anchorEl); + const uniqueId = React.useId(); + const popoverId = `filter-panel-${uniqueId}`; + const headingId = `filter-panel-heading-${uniqueId}`; - const handleOpen = (event: React.MouseEvent) => { - setAnchorEl(event.currentTarget); - }; + const handleOpen = (event: React.MouseEvent) => { + setAnchorEl(event.currentTarget); + }; - const handleClose = () => { - setAnchorEl(null); - }; + const handleClose = () => { + setAnchorEl(null); + }; - return ( - <> - {/* Trigger button */} - - - - - {/* Popover panel */} - - {/* Header */} - - Filters - {onClear && activeCount > 0 && ( - { - onClear(); - }} - underline="hover" - sx={{ fontSize: '0.8125rem' }} - > - Clear all - - )} - - - - - {/* Filter controls */} - - {children} - - - - - {/* Footer — done button */} - - - - - ); -}; + + {/* Popover panel */} + + {/* Header */} + + + {label} + + {onClear && activeCount > 0 && ( + { + onClear(); + }} + underline="hover" + sx={{ fontSize: (theme: Theme) => theme.typography.caption.fontSize }} + > + Clear all + + )} + + + + + {/* Filter controls */} + + {children} + + + + + {/* Footer — done button */} + + + + + + ); + }, +); FilterPanel.displayName = 'FilterPanel'; export default FilterPanel; diff --git a/src/components/organisms/ArrangementDialog/ArrangementDialog.tsx b/src/components/organisms/ArrangementDialog/ArrangementDialog.tsx index e23cc91..de1eb42 100644 --- a/src/components/organisms/ArrangementDialog/ArrangementDialog.tsx +++ b/src/components/organisms/ArrangementDialog/ArrangementDialog.tsx @@ -157,385 +157,412 @@ function getAuthCTALabel(subStep: AuthSubStep): string { * * Pure presentation component — props in, callbacks out. */ -export const ArrangementDialog: React.FC = ({ - open, - onClose, - step, - onStepChange, - provider, - selectedPackage, - nextSteps = DEFAULT_NEXT_STEPS, - isPrePlanning = false, - onExplore, - authValues, - onAuthChange, - authErrors, - onGoogleSSO, - onMicrosoftSSO, - onContinue, - loading = false, - sx, -}) => { - const isEmailOnly = authValues.contactPreference === 'email_only'; +export const ArrangementDialog = React.forwardRef( + ( + { + open, + onClose, + step, + onStepChange, + provider, + selectedPackage, + nextSteps = DEFAULT_NEXT_STEPS, + isPrePlanning = false, + onExplore, + authValues, + onAuthChange, + authErrors, + onGoogleSSO, + onMicrosoftSSO, + onContinue, + loading = false, + sx, + }, + ref, + ) => { + const isEmailOnly = authValues.contactPreference === 'email_only'; + const titleRef = React.useRef(null); - const handleAuthField = (field: keyof AuthValues, value: string) => { - onAuthChange({ ...authValues, [field]: value }); - }; + // Focus the dialog title when step changes + React.useEffect(() => { + if (open && titleRef.current) { + titleRef.current.focus(); + } + }, [step, open]); - return ( - - {/* ─── Header ─── */} - { + onAuthChange({ ...authValues, [field]: value }); + }; + + return ( + - - {step === 'auth' && ( - onStepChange('preview')} - aria-label="Back to preview" - > - - - )} - - {step === 'preview' ? 'Your selected package' : 'Save your plan'} - + {/* ─── Header ─── */} + + + {step === 'auth' && ( + onStepChange('preview')} + aria-label="Back to preview" + > + + + )} + + {step === 'preview' ? 'Your selected package' : 'Save your plan'} + + + + + - - - - - - {/* ═══════════ Step 1: Preview ═══════════ */} - {step === 'preview' && ( - - {/* Provider */} - - - + {/* Screen reader step announcement */} + + {step === 'preview' ? 'Viewing package preview' : 'Create your account'} + - {/* Package summary */} - + + {/* ═══════════ Step 1: Preview ═══════════ */} + {step === 'preview' && ( + + {/* Provider */} + + + + + {/* Package summary */} + + + {selectedPackage.name} + + ${(selectedPackage.total ?? selectedPackage.price).toLocaleString('en-AU')} + + + + {selectedPackage.sections.map((section) => ( + + + {section.heading} + + {section.items.map((item) => ( + + {item.name} + + ))} + + ))} + + + + You'll be able to customise everything in the next steps. + + + {/* What's next */} + + + What happens next + + + {nextSteps.map((s) => ( + + + + {s.number} + + + + + ))} + + + + + + {/* CTAs */} - {selectedPackage.name} - - ${(selectedPackage.total ?? selectedPackage.price).toLocaleString('en-AU')} - + {isPrePlanning && onExplore && ( + + )} + - - {selectedPackage.sections.map((section) => ( - - - {section.heading} - - {section.items.map((item) => ( - - {item.name} - - ))} - - ))} + )} - - You'll be able to customise everything in the next steps. - - - {/* What's next */} - - - What happens next - - - {nextSteps.map((s) => ( - - - - {s.number} - - - - - ))} - - - - - - {/* CTAs */} + {/* ═══════════ Step 2: Auth ═══════════ */} + {step === 'auth' && ( { + e.preventDefault(); + if (!loading) onContinue(); }} > - {isPrePlanning && onExplore && ( - - )} - - - - )} - - {/* ═══════════ Step 2: Auth ═══════════ */} - {step === 'auth' && ( - { - e.preventDefault(); - if (!loading) onContinue(); - }} - > - - {isPrePlanning - ? 'Save your plan to return and update it anytime.' - : 'We need a few details so a funeral arranger can help you with the next steps.'} - - - {/* SSO buttons */} - - - - - - - - or + + {isPrePlanning + ? 'Save your plan to return and update it anytime.' + : 'We need a few details so a funeral arranger can help you with the next steps.'} - - {/* Email */} - handleAuthField('email', e.target.value)} - error={!!authErrors?.email} - helperText={authErrors?.email} - placeholder="you@example.com" - autoComplete="email" - inputMode="email" - fullWidth - required - disabled={authValues.subStep !== 'email'} - sx={{ mb: 3 }} - /> - - {/* Details (after email) */} - + {/* SSO buttons */} - - A few details to save your plan - + + + + + + + or + + + + {/* Email */} + handleAuthField('email', e.target.value)} + error={!!authErrors?.email} + helperText={authErrors?.email} + placeholder="you@example.com" + autoComplete="email" + inputMode="email" + fullWidth + required + disabled={authValues.subStep !== 'email'} + sx={{ mb: 3 }} + /> + + {/* Details (after email) */} + + + + A few details to save your plan + + + + handleAuthField('firstName', e.target.value)} + error={!!authErrors?.firstName} + helperText={authErrors?.firstName} + autoComplete="given-name" + fullWidth + required + disabled={authValues.subStep === 'verify'} + /> + handleAuthField('lastName', e.target.value)} + error={!!authErrors?.lastName} + helperText={authErrors?.lastName} + autoComplete="family-name" + fullWidth + required + disabled={authValues.subStep === 'verify'} + /> + - handleAuthField('firstName', e.target.value)} - error={!!authErrors?.firstName} - helperText={authErrors?.firstName} - autoComplete="given-name" + label={isEmailOnly ? 'Phone (optional)' : 'Best number to reach you'} + type="tel" + value={authValues.phone} + onChange={(e) => handleAuthField('phone', e.target.value)} + error={!!authErrors?.phone} + helperText={authErrors?.phone} + placeholder="e.g. 0412 345 678" + autoComplete="tel" + inputMode="tel" fullWidth - required + required={!isEmailOnly} disabled={authValues.subStep === 'verify'} /> + handleAuthField('lastName', e.target.value)} - error={!!authErrors?.lastName} - helperText={authErrors?.lastName} - autoComplete="family-name" + select + label="Contact preference" + value={authValues.contactPreference} + onChange={(e) => handleAuthField('contactPreference', e.target.value)} + fullWidth + disabled={authValues.subStep === 'verify'} + > + Call me anytime + Email is preferred + Only contact by email + + + + + {/* Verification */} + + + + We've sent a 6-digit code to {authValues.email}. Please + enter it below. + + + handleAuthField('verificationCode', e.target.value)} + error={!!authErrors?.verificationCode} + helperText={ + authErrors?.verificationCode || 'Check your email for the 6-digit code' + } + placeholder="Enter 6-digit code" + autoComplete="one-time-code" + inputMode="numeric" fullWidth required - disabled={authValues.subStep === 'verify'} /> + - handleAuthField('phone', e.target.value)} - error={!!authErrors?.phone} - helperText={authErrors?.phone} - placeholder="e.g. 0412 345 678" - autoComplete="tel" - inputMode="tel" - fullWidth - required={!isEmailOnly} - disabled={authValues.subStep === 'verify'} - /> - - handleAuthField('contactPreference', e.target.value)} - fullWidth - disabled={authValues.subStep === 'verify'} + {/* Terms */} + + By continuing, you agree to the{' '} + - Call me anytime - Email is preferred - Only contact by email - + terms and conditions + + . + + + + + {/* CTA */} + + - - - {/* Verification */} - - - - We've sent a 6-digit code to {authValues.email}. Please - enter it below. - - - handleAuthField('verificationCode', e.target.value)} - error={!!authErrors?.verificationCode} - helperText={ - authErrors?.verificationCode || 'Check your email for the 6-digit code' - } - placeholder="Enter 6-digit code" - autoComplete="one-time-code" - inputMode="numeric" - fullWidth - required - /> - - - - {/* Terms */} - - By continuing, you agree to the{' '} - - terms and conditions - - . - - - - - {/* CTA */} - - - - )} - - - ); -}; + )} + + + ); + }, +); ArrangementDialog.displayName = 'ArrangementDialog'; export default ArrangementDialog; diff --git a/src/components/pages/CoffinDetailsStep/CoffinDetailsStep.tsx b/src/components/pages/CoffinDetailsStep/CoffinDetailsStep.tsx index e292445..68b0ac7 100644 --- a/src/components/pages/CoffinDetailsStep/CoffinDetailsStep.tsx +++ b/src/components/pages/CoffinDetailsStep/CoffinDetailsStep.tsx @@ -161,6 +161,13 @@ export const CoffinDetailsStep: React.FC = ({ {/* CTAs */} { + e.preventDefault(); + if (!loading) onContinue(); + }} sx={{ display: 'flex', justifyContent: 'space-between', @@ -170,13 +177,13 @@ export const CoffinDetailsStep: React.FC = ({ }} > {onSaveAndExit ? ( - ) : ( )} - diff --git a/src/components/pages/ProvidersStep/ProvidersStep.tsx b/src/components/pages/ProvidersStep/ProvidersStep.tsx index 8811446..860b6d6 100644 --- a/src/components/pages/ProvidersStep/ProvidersStep.tsx +++ b/src/components/pages/ProvidersStep/ProvidersStep.tsx @@ -145,8 +145,8 @@ export const ProvidersStep: React.FC = ({ zIndex: 1, bgcolor: 'background.default', pb: 1, - mx: -3, - px: 3, + mx: { xs: -2, md: -3 }, + px: { xs: 2, md: 3 }, }} > diff --git a/src/components/pages/VenueStep/VenueStep.tsx b/src/components/pages/VenueStep/VenueStep.tsx index 35f621c..83dc703 100644 --- a/src/components/pages/VenueStep/VenueStep.tsx +++ b/src/components/pages/VenueStep/VenueStep.tsx @@ -214,8 +214,8 @@ export const VenueStep: React.FC = ({ zIndex: 1, bgcolor: 'background.default', pb: 1, - mx: -3, - px: 3, + mx: { xs: -2, md: -3 }, + px: { xs: 2, md: 3 }, }} >