feat(feedback): missing record form improvements#836
feat(feedback): missing record form improvements#836thostetler wants to merge 8 commits intoadsabs:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Risk summary: Moderate risk. Two newly introduced no-unused-vars lint failures will likely break CI (PubDateField.tsx, RecordPanel.tsx). Draft persistence also has a couple of unhandled localStorage failure modes.
Improves the Missing Record feedback form UX by adding draft persistence, a completion checklist, and a guided step-by-step wizard.
Changes:
- Added debounced localStorage draft auto-save/restore plumbing (with a restore banner).
- Added Guided wizard mode + Standard/Guided toggle, plus a required-fields checklist in Standard mode.
- Updated form validation behavior (onTouched/onBlur) and publication date input filtering.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds new LocalSettings keys for feedback draft + form mode. |
| src/components/FeedbackForms/MissingRecord/useFormDraft.ts | New hook to read/write/clear debounced drafts in localStorage. |
| src/components/FeedbackForms/MissingRecord/useFormDraft.test.ts | Adds unit tests for draft hook behavior. |
| src/components/FeedbackForms/MissingRecord/types.ts | Extracts COLLECTIONS constant and refines types. |
| src/components/FeedbackForms/MissingRecord/ReferencesField.tsx | Adds imperative flush() support via forwardRef for wizard navigation. |
| src/components/FeedbackForms/MissingRecord/RecordWizard.tsx | New guided stepper UI with per-step validation gating. |
| src/components/FeedbackForms/MissingRecord/RecordWizard.test.tsx | Tests wizard navigation/validation with heavy component mocking. |
| src/components/FeedbackForms/MissingRecord/RecordPanel.tsx | Integrates draft banner, form mode toggle, checklist, and guided wizard into main panel. |
| src/components/FeedbackForms/MissingRecord/RecordPanel.test.tsx | Adds coverage for required-fields validity gating and edit-mode bibcode behavior. |
| src/components/FeedbackForms/MissingRecord/PubDateField.tsx | Adds input filtering + setValue-driven updates for pub date. |
| src/components/FeedbackForms/MissingRecord/FormChecklist.tsx | New live completion checklist for required fields. |
| src/components/FeedbackForms/MissingRecord/FormChecklist.test.tsx | Tests checklist completeness/progress count behavior. |
| src/components/FeedbackForms/MissingRecord/DraftBanner.tsx | New restore/dismiss banner component. |
| src/components/FeedbackForms/MissingRecord/DraftBanner.test.tsx | Tests banner rendering and callbacks. |
| src/components/FeedbackForms/MissingRecord/AuthorsTable.tsx | Adds imperative flush() support via forwardRef for wizard navigation. |
| src/components/FeedbackForms/MissingRecord/AuthorsField.tsx | Updates field wrapper to forward refs to AuthorsTable and adjusts required UI. |
| localStorage.setItem(key, JSON.stringify(values)); | ||
| setHasDraft(true); |
There was a problem hiding this comment.
localStorage.setItem() can throw (quota exceeded, storage disabled, Safari private mode). Right now that exception would bubble and break the form. Consider wrapping the write in try/catch (and only setting hasDraft=true on success) so draft persistence degrades gracefully.
| localStorage.setItem(key, JSON.stringify(values)); | |
| setHasDraft(true); | |
| try { | |
| localStorage.setItem(key, JSON.stringify(values)); | |
| setHasDraft(true); | |
| } catch { | |
| // Ignore storage write failures so the form continues to work without draft persistence. | |
| } |
| if (key === null || typeof window === 'undefined') { | ||
| return; | ||
| } | ||
| localStorage.removeItem(key); |
There was a problem hiding this comment.
localStorage.removeItem() can throw (e.g., storage unavailable). As written, clearDraft could crash the form during submission/reset. Wrap removeItem in try/catch so clearing the draft is best-effort.
| localStorage.removeItem(key); | |
| try { | |
| localStorage.removeItem(key); | |
| } catch { | |
| // Best-effort cleanup only; storage can be unavailable in some browsers/modes. | |
| } |
| const { onChange, ...rest } = register('pubDate'); | ||
|
|
||
| return ( | ||
| <FormControl isRequired isInvalid={!!errors.pubDate}> | ||
| <FormLabel>Publication Date</FormLabel> | ||
| <Input {...register('pubDate')} placeholder="yyyy-mm-dd" /> | ||
| <FormErrorMessage>{errors.pubDate && errors.pubDate.message}</FormErrorMessage> | ||
| <Input | ||
| {...rest} | ||
| placeholder="YYYY-MM" | ||
| maxLength={10} | ||
| onChange={(e) => { | ||
| e.target.value = e.target.value.replace(/[^\d-]/g, ''); | ||
| setValue('pubDate', e.target.value, { shouldValidate: true, shouldDirty: true }); | ||
| }} |
There was a problem hiding this comment.
const { onChange, ...rest } = register('pubDate') leaves an unused onChange binding, which will fail linting under @typescript-eslint/no-unused-vars. Either remove the destructuring, rename to _onChange, or incorporate it (e.g., filter the value and then call the registered onChange / use setValueAs in register options).
| function getDraftKey(isNew: boolean, bibcode?: string): string | null { | ||
| if (isNew) { | ||
| return LocalSettings.FEEDBACK_DRAFT_NEW; | ||
| } | ||
| if (bibcode) { | ||
| return `feedback-draft:edit-record:${bibcode}`; | ||
| } |
There was a problem hiding this comment.
getDraftKey uses the bibcode prop to decide the edit-record draft key. In the Edit Record tab, users can type a bibcode into the form without it being present in the URL query, so bibcode prop remains undefined and draft persistence is disabled (null key). If draft persistence is intended for edit mode, consider deriving the key from the current form value (e.g., watch/getValues('bibcode')) once a bibcode is present, and be careful not to churn keys on every keystroke (only enable when the bibcode is valid/stable).
| function getDraftKey(isNew: boolean, bibcode?: string): string | null { | |
| if (isNew) { | |
| return LocalSettings.FEEDBACK_DRAFT_NEW; | |
| } | |
| if (bibcode) { | |
| return `feedback-draft:edit-record:${bibcode}`; | |
| } | |
| function getStableBibcodeDraftValue(bibcode?: string): string | null { | |
| const normalizedBibcode = bibcode?.trim(); | |
| if (!normalizedBibcode) { | |
| return null; | |
| } | |
| return normalizedBibcode; | |
| } | |
| function getDraftKey(isNew: boolean, bibcode?: string): string | null { | |
| if (isNew) { | |
| return LocalSettings.FEEDBACK_DRAFT_NEW; | |
| } | |
| const stableBibcode = getStableBibcodeDraftValue(bibcode); | |
| if (stableBibcode) { | |
| return `feedback-draft:edit-record:${stableBibcode}`; | |
| } |
| <Controller | ||
| name="collection" | ||
| control={control} | ||
| render={({ field: { ref, ...rest } }) => ( |
There was a problem hiding this comment.
In the Controller render for collection, the destructured ref is unused. With @typescript-eslint/no-unused-vars enabled, this will fail lint. Rename it to _ref (as done elsewhere) or omit it entirely.
| render={({ field: { ref, ...rest } }) => ( | |
| render={({ field: { ref: _ref, ...rest } }) => ( |
…izard for missing record form
6cbf6a3 to
da19be2
Compare
…lders for URLs and references
… example.com are accepted
…mode preview/submit
… captures full input not just first character
…orrectly fail URL validation
Improves the missing record submission UX.