feat: add loading spinner during API calls#291
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors authentication and task flows to centralize async logic and surface loading state via a shared Spinner component.
Changes:
- Adds
login/signuptoAuthContextwith a sharedloadingstate, and consumes them fromLogin/Signuppages. - Adds
loadingtouseTaskshook and renders aSpinnerin the Tasks list while fetching. - Introduces a new reusable
Spinnercomponent.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/Spinner.jsx | New reusable spinner component. |
| frontend/src/context/AuthContext.jsx | Adds login, signup, and shared loading state to context. |
| frontend/src/hooks/useTasks.js | Tracks loading state across fetch/add/update/delete. |
| frontend/src/pages/Login.jsx | Uses context login; shows spinner; disables button while loading. |
| frontend/src/pages/Signup.jsx | Uses context signup; shows spinner; disables button while loading. |
| frontend/src/pages/Tasks.jsx | Renders Spinner while tasks are loading. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setLoading(true); | ||
| await api.post("/tasks", taskData); | ||
| getTasks(); | ||
| await getTasks(); | ||
| }; |
| const addTask = async (taskData) => { | ||
| setLoading(true); | ||
| await api.post("/tasks", taskData); | ||
| getTasks(); | ||
| await getTasks(); | ||
| }; |
| <div className="grid grid-cols-1 lg:grid-cols-3 gap-6"> | ||
| <div className="lg:col-span-2 space-y-4 animate-in delay-200"> | ||
| {tasks.length ? ( | ||
| {loading ? ( |
| logout(); | ||
| }); | ||
| } | ||
| }, [token]); | ||
|
|
||
| return ( | ||
| <AuthContext.Provider value={{ user, token, setUser, setToken, logout }}> | ||
| <AuthContext.Provider value={{ user, token, setUser, setToken, logout, login, signup, loading }}> |
| const login = async (email, password) => { | ||
| setLoading(true); | ||
| try { | ||
| const res = await api.post("/auth/login", { email, password }); | ||
| setToken(res.data.token); | ||
| setUser(res.data.user); | ||
| localStorage.setItem("token", res.data.token); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; |
| await login(email, password); | ||
| navigate("/dashboard"); | ||
| } catch (error) { | ||
| // handle error | ||
| console.log("Login failed"); | ||
| console.log(error.response?.data || error.message); | ||
| } |
| const Spinner = () => { | ||
| return ( | ||
| <div className="flex justify-center items-center py-8"> | ||
| <div className="w-8 h-8 border-4 border-blue-500 border-t-transparent rounded-full animate-spin"></div> |
| const Spinner = () => { | ||
| return ( | ||
| <div className="flex justify-center items-center py-8"> | ||
| <div className="w-8 h-8 border-4 border-blue-500 border-t-transparent rounded-full animate-spin"></div> |
| // restore session on app load | ||
| useEffect(() => { | ||
| if (token) { | ||
| // fetch logged-in user | ||
| api | ||
| .get("/auth/me") | ||
| .then((res) => { | ||
| setUser(res.data.user); |
|
@bh462007 pls fix the linting errors and dont use AI for your task or else you will be banned from the competition |
|
Hi @aryandas2911, thanks for pointing it out. I will fix the linting errors and go through the changes properly before updating the PR. |
…462007/DailyForge into fix/loading-spinner-api-calls
…o fix/loading-spinner-api-calls
|
Hi @aryandas2911, I’ve fixed the AuthContext linting issue by separating the context and provider into different files. For |
|
@bh462007 kindly check the merge conflicts and fix them |
|
Hi @aryandas2911, I noticed that issue #1 has been closed while my PR #291 is still open. I’ve been working on the requested changes, including resolving merge conflicts and checking the linting issues. Could you please let me know whether the PR is still being considered or if I should close it? Thanks! |
Closes #1
What Changed
useTasks.jsSpinnercomponentAuthContextfor login/signup flowsFiles Changed
src/hooks/useTasks.jssrc/components/Spinner.jsxsrc/pages/Tasks.jsxsrc/context/AuthContext.jsxsrc/pages/Login.jsxsrc/pages/Signup.jsx