MPDX-9763 - NSO Questionnaire: Improve text field designs (while still honoring Figma design)

[zweatshirt]

Description

• With the current Figma design, all of the questions exist inside of the fields.
  • This is a great design, but there are some complications around the start adornments overlapping with the labels (we can add custom logic surrounding that to fix this, I'm just on the fence about doing this)
• I am wondering if we would prefer to have Typography labels above the text fields instead. I think this would diminish the cool design but honestly make the form more readable.

Future considerations:

• We may also want to include the dollar sign start adornment for some of these.

Testing

• Go to NSO Questionnaire
• Check the steps and observe the appearance of the form fields.
• Feel free to give an opinion.

Checklist:

• I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
• I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
• I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
• I have requested a review from another person on the project
• I have tested my changes in preview or in staging
• I have cleaned up my commit history

[github-actions]

Bundle sizes [mpdx-react]

Compared against b969697

No significant changes found

[github-actions]

Preview branch generated at https://improve-questionnaire-readability.d3dytjb8adxkk5.amplifyapp.com

[zweatshirt]

🤖 Multi-Agent Code Review — Verdict: ✅ APPROVED WITH SUGGESTIONS

Risk: 2/10 (LOW) · 5 agents (Architecture, Testing, Standards, UX, Financial) + dependency analysis · standard mode

No blockers. This is a clean styling/refactor PR: NumberQuestion moves from a hand-wired FormControl+OutlinedInput to a single MUI TextField (net debt reduction), and field widths become responsive. The prior review's one Important finding (non-responsive width: '50%') is resolved via width: '100%' + maxWidth: theme.spacing(60). Accessibility is a net improvement (real <label>, auto aria-describedby, aria-invalid); no breaking changes (props interface unchanged, 3 consumers safe); Standards clean; no financial code.

Findings (all non-blocking)

#	Sev	File	Issue
1	6.0 Medium	MinistryDetails.tsx:108	Location field still uses placeholder only, no label — the exact bug this PR fixes elsewhere
2	6.5 Medium	MinistryDetails.tsx:122 (+NsoDetails)	Longest labels still truncate at 480px — mitigated, not fully resolved
3	5.0 Medium	NumberQuestion.tsx:39 (+3 sites)	Repeated width sx; a third width value in the tree — consider a shared token
4	4.0 Suggestion	NumberQuestion.test.tsx:54	New label test doesn't assert placeholder absence

See inline comments for detail.

---

Findings on Related Files (Not in This PR)

These were surfaced on files related to the change but not modified in this PR, so they can't be posted as line comments. Informational only.

[Suggestion] src/components/HrTools/NsoMpdQuestionnaire/FinancialInformation/FinancialDetails.tsx:100 — Adorned vs non-adorned label-shrink inconsistency

• Severity: 3.5/10 · Flagged by: UX
• The debt fields pass a startAdornment icon, which makes MUI shrink the NumberQuestion label immediately, while the NsoDetails fields (no adornment) rest the label full-size until focus/fill — two visual states for the same component. Optional: set InputLabelProps={{ shrink: true }} on NumberQuestion to unify.

[Medium] src/components/HrTools/NsoMpdQuestionnaire/NsoInformation/NsoDetails.tsx:67-69 — Long-label truncation (companion to inline finding #2)

• Severity: 6.5/10 · Flagged by: UX
• "How much special needs support have you already received for NSO?" (~65 chars) still clips at 480px. Structural fix (follow-up): render long questions as a Typography above the field with a short label — the pattern RadioQuestion and ContactInformation already use in this tree.

[zweatshirt]

[Medium] This location field still uses `placeholder` as its only question text, with no `label` (and `InputLabelProps={{ shrink: true }}` is inert with no label to shrink). It therefore has the exact problem this PR fixes everywhere else: the question disappears as soon as the user types, and the field has no accessible name once filled. Its two sibling `select` fields correctly use `label=`. Suggest converting:

<TextField
  label={t('What is your expected ministry assignment location?')}
  sx={{ width: '100%', maxWidth: theme.spacing(60) }}
  size="small"
  {...locationProps}
/>

(This surfaces the same long-label truncation tradeoff as the city field, but resolves the disappearing-question + a11y gap.)

[zweatshirt]

[Medium] The responsive width fix (`width: '100%'` + `maxWidth: theme.spacing(60)` = 480px) is good and resolves the prior non-responsive concern. However, a shrunk MUI outlined label fits ~55–65 chars at 480px, so the longest question here — *"Is your ministry assignment location within 50 miles of one of these cities?"* (~74 chars) — still truncates with an ellipsis. Substantially mitigated, not fully resolved. Acceptable to ship; the structural fix (follow-up) is to render long questions as a `Typography` above the field with a short `label`, matching the `RadioQuestion`/`ContactInformation` pattern already used in this feature tree.

[zweatshirt]

[Medium] `sx={{ width: '100%', maxWidth: theme.spacing(60) }}` is now repeated across 4 sites (here + the 3 `MinistryDetails` fields), and the questionnaire tree now has two field-width treatments — 480px here vs 320px (`theme.spacing(40)`) in `ContactInformation`. As more question components land this magic value will drift. Consider hoisting a shared `questionFieldSx` (or a small styled `TextField`) into the `Shared/` folder. If you consolidate, fold `ContactInformation` in too rather than leaving a third variant. Non-blocking for a 3-file PR.

[zweatshirt]

[Suggestion] This new test and the existing `'renders the numeric input'` test (`getByRole('spinbutton', { name: 'How many?' })`) both resolve through the same label→input association, so they prove the same thing. It also doesn't fully lock the placeholder→label migration this PR makes — a regression back to `placeholder={question}` wouldn't fail it. Add the negative assertion to actually guard the intent:

expect(queryByPlaceholderText('How many?')).not.toBeInTheDocument();

[github-actions]

AI Review Auto-Approval

Risk Level: LOW (2/10)
Verdict: APPROVED_WITH_SUGGESTIONS (suggestions posted, no blockers)

This PR was auto-approved because:

• The multi-agent AI review determined it is low risk
• No blocking issues were found
• All suggestions have been posted as review comments for the developer to consider

If you believe this PR needs human review, dismiss this approval and request a review manually.