[CORE-229] Fix multiple file upload issues#33
Merged
elvisdragonmao merged 3 commits intomainfrom Mar 9, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple file upload issues in a forms application, renames FormDetailPage to FormFilloutPage, and refactors shared UI into reusable components. It also moves OAuth account-binding logic from the page level into the AccountButton component itself, and fixes the FormData field name from "file" to "files" to match the server API.
Changes:
- Fixes file upload by correcting the FormData field name, implementing batch upload logic (replace-all strategy), and adding support for re-loading previously uploaded files from the server
- Moves OAuth popup handling from
FormFilloutPageintoAccountButton, making the component self-contained with its own fetch, polling, and state management - Extracts
FormHeader,FormStructure, andFormPreviewSectioninto shared sub-components to reduce duplication betweenFormFilloutPageandAdminFormPreviewPage
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/shared/components/FileUpload/FileUpload.tsx |
Wraps non-image file preview in a styled <div> instead of a fragment |
src/shared/components/AccountButton/AccountButton.tsx |
Adds self-contained OAuth connect logic with popup polling, avatar display, and toast notifications |
src/shared/components/AccountButton/AccountButton.module.css |
Adds avatar styling for connected account display |
src/routes/UserRouter.tsx |
Renames lazy import from FormDetailPage to FormFilloutPage |
src/features/form/services/api.ts |
Fixes FormData field name to "files", adds clearQuestionFiles, uses proper type imports |
src/features/form/components/index.ts |
Updates export from FormDetailPage to FormFilloutPage |
src/features/form/components/FormQuestionRenderer.tsx |
Overhauls file upload with batch upload, server file initialization, metadata tracking, and ServerFileInfo type |
src/features/form/components/FormFilloutPage.tsx |
Renames component, uses extracted sub-components, adds file metadata state, removes inline OAuth/preview logic |
src/features/form/components/FormFilloutPage.module.css |
Removes header/structure/legend CSS (now in extracted component CSS files) |
src/features/form/components/FormDetail/components/FormStructure/FormStructure.tsx |
New shared component for form section navigation with completion state |
src/features/form/components/FormDetail/components/FormStructure/FormStructure.module.css |
Styles for form structure component |
src/features/form/components/FormDetail/components/FormPreviewSection/FormPreviewSection.tsx |
New shared component for preview with "response" and "local" modes |
src/features/form/components/FormDetail/components/FormPreviewSection/FormPreviewSection.module.css |
Styles for preview section component |
src/features/form/components/FormDetail/components/FormHeader/FormHeader.tsx |
New shared component for form header with save status indicator |
src/features/form/components/FormDetail/components/FormHeader/FormHeader.module.css |
Styles for form header component |
src/features/form/components/AdminFormPreviewPage.tsx |
Refactored to use extracted FormHeader, FormStructure, FormPreviewSection |
src/features/form/components/AdminFormDetailPage.tsx |
Adds copy-link button and conditionally shows preview vs view buttons based on form status |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+111
to
+120
| timerRef.current = window.setInterval(async () => { | ||
| if (popup.closed) { | ||
| clearTimer(); | ||
| await fetchAndApply({ showToast: true }); | ||
| setIsConnecting(false); | ||
| } | ||
| }, 500); | ||
|
|
||
| popup.focus(); | ||
| }; |
There was a problem hiding this comment.
Bug: There is no cleanup effect to clear the interval timer (timerRef) when the component unmounts. If a user navigates away while the OAuth popup is open, the setInterval callback will continue to run and attempt to call setState (setIsConnecting, setResolvedLabel, setAvatarUrl) on an unmounted component. The old code in FormFilloutPage had this cleanup in its useEffect return. Add a useEffect with a cleanup function that calls clearTimer() on unmount.
elvisdragonmao
approved these changes
Mar 9, 2026
elvisdragonmao
force-pushed
the
fix/CORE-229-file-upload-issues
branch
from
4540a80 to
0eaa703
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of changes
Purpose