feat: implement Suspense-based loading states across application pages - #54
Conversation
|
Someone is attempting to deploy a commit to the participationcorner2025-8967's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Codesense Ai: This PR is too large to review automatically. A human maintainer will take a look! |
|
Hey @Abhinav1190P! Excellent work here. Using I just finished reviewing the code and everything looks perfect. However, we just merged PR #55 (HTTP Edge Caching) which added a Could you please Once you push the resolved branch, I will merge this immediately! |
rushikesh-bobade
changed the title
|
Hey @Abhinav1190P, thanks for the PR! We recently made some massive changes to the main branch to generalize the database schema and UI components. As a result, this PR now has merge conflicts. Could you please fetch the latest main branch, rebase your changes (or merge main into your branch), and resolve the conflicts? Let me know once that's done! |
📝 SummaryThe provided pull request introduces new skeleton components for the dashboard, inventory management, and sales log pages. These components are designed to display a loading state while data is being fetched. The changes also update the respective route files to utilize these new skeleton components. 📂 Files Changed
🎭 Code PoemSkeletons born, to load with care, 🚨 Bugs & Architectural Violations* The code seems to follow the architectural rules, using Remix and Prisma as required. * The use of Vanilla CSS Modules is consistent throughout the changes. * All UI components are placed in the correct `app/blocks/` directory. * No Tailwind CSS classes are used, adhering to the rules. * However, it's worth noting that the `Skeleton` component is used extensively, and its implementation should be reviewed to ensure it follows the performance guidelines. * The `dashboard-skeleton.tsx` file uses inline styles, which might not be the best practice. Consider moving these styles to a separate CSS module file. * The `inventory-table-skeleton.tsx` and `sales-log-skeleton.tsx` files use CSS modules correctly. * No accessibility issues are immediately apparent, but a thorough review of the components' accessibility features, such as `aria-hidden` and `roles`, is recommended.💡 Suggestions & Best Practices* Consider adding a `defer` function to the `Skeleton` component to improve performance. * Review the `Skeleton` component's implementation to ensure it uses `transform` or `opacity` for CSS animations, rather than triggering main-thread repaints. * Add accessibility features, such as `aria-hidden` and `roles`, to the skeleton components to improve their accessibility. * Consider using a more robust loading state management system, rather than relying on a simple `Skeleton` component. * Review the code for any potential performance bottlenecks, such as unnecessary re-renders or slow data fetching. * Consider adding error handling and edge cases to the skeleton components to ensure they behave correctly in unexpected situations. |
|
Yeah i can add skeleton back later, since it is low priority |
|
Hey @Abhinav1190P! Thanks for pushing that merge! It looks like during the merge conflict resolution, the original code from Don't worry, merge conflicts can be tricky! To fix this, you just need to wrap your Here is exactly how you can update 1. In the const dashboardPromise = Promise.all([
// ... your prisma queries ...
]).then(([inventoryStats, salesData, expensesData]) => {
// ... your formatting logic ...
return { inventoryStats: serializedStats, salesData: serializedSales, expensesData: serializedExpenses };
});
return { dashboardData: dashboardPromise };2. In the import { DashboardSkeleton } from "~/blocks/dashboard/dashboard-skeleton";
export default function DashboardPage() {
const { dashboardData } = useLoaderData<typeof loader>();
return (
<div className={styles.page}>
<DashboardHeader />
<AIInsightsPanel />
<Suspense fallback={<DashboardSkeleton />}>
<Await resolve={dashboardData}>
{({ inventoryStats, salesData, expensesData }) => (
<>
{/* ... render the charts and tables using the destructured data here ... */}
</>
)}
</Await>
</Suspense>
</div>
);
}You'll just need to apply this same pattern to |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions! |
|
Hey @Abhinav1190P, thanks for continuing the great work from Part 1! The skeleton components themselves are well-built and reuse your ✅ What's Great
🔴 Critical Issues — Must Fix Before Merge1. This is the biggest issue. You imported For example, in
The JSX needs to be wrapped like this: <Suspense fallback={<DashboardSkeleton />}>
<Await resolve={dashboardData}>
{({ inventoryStats, salesData, expensesData }) => (
<>
<StatsCardsRow stats={inventoryStats} sales={salesData} expenses={expensesData} />
{/* ... rest of the dashboard ... */}
</>
)}
</Await>
</Suspense>The same problem exists in 2. Dashboard Loader Still Awaits Everything Synchronously Even though you wrapped the return in To actually get streaming, you need to return the un-awaited promise: const dashboardData = Promise.all([inventoryQuery, salesQuery, expenseQuery])
.then(([inventoryStats, salesData, expensesData]) => ({
inventoryStats: serialize(inventoryStats),
salesData: serialize(salesData),
expensesData: serialize(expensesData),
}));
return { dashboardData }; // Don't await it!3. Stray Character in <table className={styles.table}>ß // <-- stray "ß" characterThis 🟡 Minor4. Inline styles in 5. Missing newlines at EOF: 6. Unrelated formatting change in Summary
The skeleton components are beautifully designed, but the core deliverable of this PR — making the pages stream data with |
|
🎉 Congratulations @Abhinav1190P! 🎉 Your Pull Request has been successfully merged! Thank you so much for your hard work and contribution to FlipTrack. We really appreciate it! 🚀 |
2 participants
Description
Implemented deferred data loading using
SuspenseandAwaitfor the Dashboard, Inventory Management, and Sales Log pages. Added page-specific skeleton loaders to display placeholder content while data is being fetched, improving the overall loading experience and perceived performance.Related Issues
Fixes #35
Type of Change
Checklist:
Screenshots (if applicable)
N/A