First Implementation of Error Boundary and Toast Notification System#32
First Implementation of Error Boundary and Toast Notification System#32jayvionthaing wants to merge 1 commit into
Conversation
…on System. Includes a test panel featuring buttons that will display the desired notification (will be removed after initial feedback).
Code ReviewCannot merge — all 4 CI checks are failing, and there are a couple of things that need to be addressed in the PR itself before it's ready. What's good
Issues to fix before merge1. Remove the test panel You already flagged this — the test panel with demo buttons must be removed before this merges to main. It adds dead UI that real users would see. 2. Lint error in The CI is reporting: If the parameter is intentionally unused (e.g. an error handler that ignores the first argument), prefix it with an underscore to signal that: // Instead of:
(_, errorInfo) => { ... }
// Use:
(_error, errorInfo) => { ... }3. Pre-existing CI failures (not this PR's fault, but blocking) There are many pre-existing lint failures across the codebase ( 4. Regarding CI not showing up You mentioned not seeing CI checks on the PR. They are running — you can see them at the bottom of the PR page under "Checks". They show up a minute or two after pushing. All 4 are currently failing (Backend Lint, Backend Tests, Frontend Lint, Frontend Build). Action items
Once those two things are fixed and CI passes, this is a good addition. |
StatusCI: All 4 checks failing. No reviewer assigned. Branch is behind main. This PR needs:
Not mergeable until reviewed and CI is green. |
Includes a test panel featuring buttons that will display the desired notification (will be removed after initial feedback). I'm not sure if the CI checks are supposed to appear here in the pull request, but I don't see them here. If needed to be done manually, I can do them along with removing the test panel during the feedback and revision process!