Feature: Phase 6 - In-cell editing, bulk ops, archiving, Excel import#21
Merged
ImJustChew merged 3 commits intomainfrom Feb 6, 2026
Merged
Feature: Phase 6 - In-cell editing, bulk ops, archiving, Excel import#21ImJustChew merged 3 commits intomainfrom
ImJustChew merged 3 commits intomainfrom
Conversation
Students page: - Table now supports in-cell editing (double-click any cell to edit) - Added bulk status change via toolbar (select rows + change status) - Added Excel/CSV import dialog with column mapping for bulk updates - Added status filter dropdown to view enrolled/graduated/transferred - Pinned key columns (ID, name, class) for horizontal scrolling - Columns save directly to Firestore on edit Student detail page: - Replaced dialog-based editing with inline Edit Profile mode - Edit button toggles form fields directly on the page - Save/Cancel buttons appear in edit mode with loading state Attendance page: - Added archive/unarchive system for attendance records - Archive per-record via column menu or bulk via toolbar - View Archived dialog to browse and restore archived records - Fixed new record modal: added notes field, error handling, loading state, Cancel button, proper validation - Main view now filters out archived records automatically - Added missing key props to FormLinks list Added xlsx dependency for Excel file parsing. https://claude.ai/code/session_01ANgyoygNmZZrehJ2nC1Hms
Phase 4 - Form Component Fixes: - Fix EditStudentProfile: proper type for EditableStudentFields, remove conditional register calls (all fields always registered, disabled instead), add error states, error handling with snackbar, Cancel button, loading spinner, committeeRole field - Fix AddStudentProfile: proper defaults for all fields, mode changed to onSubmit to prevent premature validation, error display on required fields, Cancel button, loading spinner, snackbar feedback Phase 5 - Code Quality: - InventoryTree: move global state (previousOpen, nodeSnapshots) into component as useState/useRef, proper cleanup on unmount, fix == to === comparisons, remove console.log statements - useHooks: replace deprecated process.browser with typeof window check, remove console.log, fix useOnScreen cleanup - useOnClickOutside: rename from useOnClickOutisde (typo), remove @ts-nocheck, add proper TypeScript types - useAuth: replace == with === throughout, remove console.log statements (keep console.error for actual errors) - dashboard: fix missing key prop on map iteration, fix == to === - MemberLayout: fix == to === for permission checks and route matching Responsive Design (RWD): - MemberLayout: redesigned for mobile with bottom navigation bar showing top-level nav items, slide-out Drawer for full menu with user profile and logout, desktop sidebar unchanged - Login page: stack layout vertically on mobile, full-width sign-in button, responsive text sizing - Page component: smaller padding and text on mobile - FullUserProfile: responsive profile picture, stacked layout on mobile, smaller text for data rows - Student detail page: flex-col on mobile for permissions/status - Dashboard: responsive table with horizontal scroll, smaller padding - globals.css: mobile-optimized DataGrid cells, dialog padding https://claude.ai/code/session_01ANgyoygNmZZrehJ2nC1Hms
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 6 of a student management system, adding advanced table features and mobile responsive improvements. The changes introduce in-cell editing for the students table, bulk operations including status changes and Excel import, an archiving system for attendance records, and inline editing for student profiles.
Changes:
- Added in-cell editing, bulk status updates, and Excel import functionality to the students page
- Replaced dialog-based editing with inline edit mode on student detail pages
- Implemented archive/unarchive system for attendance records with filtering
- Enhanced mobile responsiveness across multiple pages with responsive navigation
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/Attendance.ts | Added archived boolean field to AttendanceRecord type |
| src/styles/globals.css | Added mobile-responsive styles for MUI DataGrid components |
| src/pages/students/index.tsx | Implemented in-cell editing, bulk operations, Excel import dialog, and status filtering |
| src/pages/students/[studentid].tsx | Minor layout improvements for responsiveness |
| src/pages/index.tsx | Enhanced mobile layout with responsive sign-in button placement |
| src/pages/dashboard.tsx | Improved table responsiveness and added missing key prop |
| src/pages/attendance/view.tsx | Added archive/unarchive functionality, bulk archiving, improved modal validation |
| src/hooks/useOnClickOutside.tsx | Created new properly-typed hook (replaces misspelled version) |
| src/hooks/useOnClickOutisde.tsx | Removed misspelled hook file |
| src/hooks/useHooks.tsx | Code cleanup and simplified error handling |
| src/hooks/useAuth.tsx | Code cleanup, removed unused imports and console logs |
| src/components/Page.tsx | Improved mobile padding |
| src/components/MemberLayout.tsx | Added mobile drawer navigation and bottom navigation bar |
| src/components/InventoryTree.tsx | Fixed ref handling with useRef instead of module-level variable |
| src/components/FullUserProfile.tsx | Implemented inline edit mode replacing dialog-based editing |
| src/components/EditStudentProfile.tsx | Enhanced error handling and loading states |
| src/components/AddStudentProfile.tsx | Enhanced error handling and loading states |
| package.json | Added xlsx dependency (v0.18.5) for Excel file parsing |
| package-lock.json | Lock file updates for new dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ─── Bulk Archive Dialog ────────────────────────────────────────────────── | ||
| const BulkArchiveDialog = ({ onClose }: { onClose: () => void }) => { | ||
| const [records = [], loading] = useCollectionData<AttendanceRecord>( | ||
| query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp', 'desc')) |
There was a problem hiding this comment.
The query for non-archived records uses where('archived', '!=', true) which requires a composite index in Firestore when combined with orderBy('archived') and orderBy('startTimestamp', 'desc'). This will fail at runtime if the index doesn't exist. Consider either creating the required composite index or simplifying the query to where('archived', '==', false) or removing one of the orderBy clauses.
Suggested change
| query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp', 'desc')) | |
| query( | |
| collection(db, "attendanceRecords").withConverter(docConverter), | |
| where('archived', '==', false), | |
| orderBy('startTimestamp', 'desc'), | |
| ) |
| const [students = [], studentsLoad, studentsError] = useCollectionData<StudentDetails>(query(collection(db, "students").withConverter(docConverter), where('status', '==', 'enrolled'))); | ||
| // const { error, data: students = []} = useAPIFetch<StudentDetails[]>('students',{}, user) | ||
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), orderBy('startTimestamp','asc'))); | ||
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp','asc'))); |
There was a problem hiding this comment.
The main attendance view query also uses where('archived', '!=', true) with multiple orderBy clauses which requires a composite index. This query will fail at runtime without the proper Firestore index configuration. Consider changing to where('archived', '==', false) or documenting the required index configuration.
Suggested change
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp','asc'))); | |
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '==', false), orderBy('archived'), orderBy('startTimestamp','asc'))); |
| } | ||
| }; | ||
| }, []); // Empty array ensures that effect is only run on mount and unmount | ||
| }, []); |
There was a problem hiding this comment.
The useOnScreen hook is missing ref and rootMargin in the useEffect dependency array. This could cause the observer to not properly update when the ref or rootMargin changes. The dependency array should include [ref, rootMargin] instead of an empty array.
Suggested change
| }, []); | |
| }, [ref, rootMargin]); |
Comment on lines
+54
to
+92
| const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const f = e.target.files?.[0]; | ||
| if (!f) return; | ||
| setFile(f); | ||
| setResult(null); | ||
|
|
||
| const reader = new FileReader(); | ||
| reader.onload = (evt) => { | ||
| const data = evt.target?.result; | ||
| const workbook = XLSX.read(data, { type: 'binary' }); | ||
| const sheetName = workbook.SheetNames[0]; | ||
| const sheet = workbook.Sheets[sheetName]; | ||
| const jsonData = XLSX.utils.sheet_to_json<ExcelRow>(sheet, { defval: '' }); | ||
|
|
||
| if (jsonData.length === 0) { | ||
| enqueueSnackbar('Excel file is empty', { variant: 'error' }); | ||
| return; | ||
| } | ||
|
|
||
| const cols = Object.keys(jsonData[0]); | ||
| setHeaders(cols); | ||
| setSheetData(jsonData); | ||
|
|
||
| // Auto-map columns by name similarity | ||
| const autoMap: Record<string, string> = {}; | ||
| cols.forEach(col => { | ||
| const lower = col.toLowerCase().replace(/[\s_-]/g, ''); | ||
| if (lower.includes('studentid') || lower === 'id' || lower === 'sid') autoMap[col] = 'studentid'; | ||
| else if (lower.includes('englishname') || lower === 'name' || lower === 'english') autoMap[col] = 'englishName'; | ||
| else if (lower.includes('chinesename') || lower === 'chinese' || lower.includes('中文')) autoMap[col] = 'chineseName'; | ||
| else if (lower === 'class' || lower.includes('班级') || lower.includes('kelas')) autoMap[col] = 'class'; | ||
| else if (lower === 'gender' || lower.includes('性别')) autoMap[col] = 'gender'; | ||
| else if (lower.includes('phone') || lower.includes('电话')) autoMap[col] = 'phone'; | ||
| else if (lower.includes('email')) autoMap[col] = 'email'; | ||
| }); | ||
| setMapping(autoMap); | ||
| }; | ||
| reader.readAsBinaryString(f); | ||
| }; |
There was a problem hiding this comment.
The Excel import feature lacks file size validation. Large Excel files could cause memory issues or freeze the browser. Consider adding a maximum file size check (e.g., 5MB) before attempting to parse the file to prevent potential denial-of-service scenarios.
Comment on lines
+131
to
+147
| const updateData: Record<string, any> = { modifiedOn: Timestamp.now() }; | ||
| for (const [col, field] of updateFields) { | ||
| const val = String(row[col] ?? '').trim(); | ||
| if (val) { | ||
| updateData[field] = field === 'englishName' ? val.toUpperCase() : val; | ||
| } | ||
| } | ||
|
|
||
| batch.update(doc(db, 'students', sid), updateData); | ||
| batchCount++; | ||
| } | ||
|
|
||
| if (batchCount > 0) { | ||
| await batch.commit(); | ||
| updated += batchCount; | ||
| } | ||
| } |
There was a problem hiding this comment.
The Excel import doesn't validate or sanitize the data before writing to Firestore. Malicious Excel files could inject arbitrary data into student records. Consider adding validation for expected data types and formats (e.g., validate email format, phone number format, ensure gender is either 'Male' or 'Female', validate date formats) before updating the database.
| const { userToken } = useAuth(); | ||
| const { register, handleSubmit, setValue, control, watch, formState: { isValid, errors }, reset } = useForm<RecordForm>({ | ||
| const { enqueueSnackbar } = useSnackbar(); | ||
| const { register, handleSubmit, setValue, control, watch, formState: { isValid, errors, isSubmitting }, reset } = useForm<RecordForm>({ |
There was a problem hiding this comment.
Unused variable isValid.
Suggested change
| const { register, handleSubmit, setValue, control, watch, formState: { isValid, errors, isSubmitting }, reset } = useForm<RecordForm>({ | |
| const { register, handleSubmit, setValue, control, watch, formState: { errors, isSubmitting }, reset } = useForm<RecordForm>({ |
| const [students = [], studentsLoad, studentsError] = useCollectionData<StudentDetails>(query(collection(db, "students").withConverter(docConverter), where('status', '==', 'enrolled'))); | ||
| // const { error, data: students = []} = useAPIFetch<StudentDetails[]>('students',{}, user) | ||
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), orderBy('startTimestamp','asc'))); | ||
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp','asc'))); |
There was a problem hiding this comment.
Unused variable recordsError.
Suggested change
| const [records = [], recordsLoad, recordsError] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp','asc'))); | |
| const [records = [], recordsLoad] = useCollectionData<AttendanceRecord>(query(collection(db, "attendanceRecords").withConverter(docConverter), where('archived', '!=', true), orderBy('archived'), orderBy('startTimestamp','asc'))); |
| import { Button } from "@mui/material"; | ||
| import { DataGridPro, GridColDef, GridToolbar } from "@mui/x-data-grid-pro"; | ||
| import { collection, query, where } from "firebase/firestore"; | ||
| import { Button, Menu, MenuItem, Chip, DialogTitle, DialogContent, DialogActions, FormControl, InputLabel, Select, Typography, Box, Alert, LinearProgress, IconButton } from "@mui/material"; |
There was a problem hiding this comment.
Unused imports Box, IconButton.
Suggested change
| import { Button, Menu, MenuItem, Chip, DialogTitle, DialogContent, DialogActions, FormControl, InputLabel, Select, Typography, Box, Alert, LinearProgress, IconButton } from "@mui/material"; | |
| import { Button, Menu, MenuItem, Chip, DialogTitle, DialogContent, DialogActions, FormControl, InputLabel, Select, Typography, Alert, LinearProgress } from "@mui/material"; |
| import { db, docConverter } from "../../config/firebase"; | ||
| import { useDialog } from "../../hooks/useDialog"; | ||
| import StudentDetails from "../../types/StudentDetails"; | ||
| import { Upload, Download } from "@mui/icons-material"; |
There was a problem hiding this comment.
Unused import Download.
Suggested change
| import { Upload, Download } from "@mui/icons-material"; | |
| import { Upload } from "@mui/icons-material"; |
| const Students = () => { | ||
| const [students = [], loading, error] = useCollectionData<StudentDetails>(query(collection(db, 'students').withConverter(docConverter), where('status', '==', 'enrolled'))); | ||
| const [statusFilter, setStatusFilter] = useState<StudentDetails['status']>('enrolled'); | ||
| const [students = [], loading, error] = useCollectionData<StudentDetails>( |
There was a problem hiding this comment.
Unused variable error.
Suggested change
| const [students = [], loading, error] = useCollectionData<StudentDetails>( | |
| const [students = []] = useCollectionData<StudentDetails>( |
Resolved conflicts in dashboard.tsx and index.tsx: - dashboard: kept main's improved UI (loading state, sorted dates, Paper cards, icons) and added RWD (responsive padding, text sizes) - index: kept main's auth loading spinner and added RWD (mobile-first layout, stacked on small screens, full-width sign-in button) https://claude.ai/code/session_01ANgyoygNmZZrehJ2nC1Hms
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.
Students page:
Student detail page:
Attendance page:
loading state, Cancel button, proper validation
Added xlsx dependency for Excel file parsing.
https://claude.ai/code/session_01ANgyoygNmZZrehJ2nC1Hms