feat: persist filters across booking tabs #21705
Conversation
|
Someone is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
consumer
enterprise
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (06/04/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (06/04/25)1 label was added to this PR based on Keith Williams's automation. |
|
Himanshu Meena seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
dosubot
bot
added
bookings
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 4 issues across 3 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| @@ -0,0 +1,68 @@ | |||
| import { useCallback, useEffect, useState } from "react"; | |||
There was a problem hiding this comment.
This hook calls the client-only Next.js hook usePathname, but the file lacks a 'use client' directive at the top. Without it, the module is treated as a server module and using usePathname will cause a build error in Next.js 13+.
| } | ||
|
|
||
| export function useSharedFilters(tableIdentifier: string) { | ||
| const pathname = usePathname(); |
There was a problem hiding this comment.
The pathname variable is declared but never used, resulting in an unused variable that will trigger ESLint warnings and increases code noise.
| const pathname = usePathname(); | |
| // const pathname = usePathname(); // removed – variable was unused |
| pageSize, | ||
| searchTerm, | ||
| }); | ||
| }, [activeFilters, sorting, columnVisibility, columnSizing, pageSize, searchTerm]); |
There was a problem hiding this comment.
setFilters is used inside the effect but not included in the dependency array, breaking the exhaustive-deps rule and risking updates being written with a stale function reference.
| }, [activeFilters, sorting, columnVisibility, columnSizing, pageSize, searchTerm]); | |
| }, [activeFilters, sorting, columnVisibility, columnSizing, pageSize, searchTerm, setFilters]); |
| if (sharedFilters.columnSizing) setColumnSizing(sharedFilters.columnSizing); | ||
| if (sharedFilters.pageSize) setPageSize(sharedFilters.pageSize); | ||
| if (sharedFilters.searchTerm) setSearchTerm(sharedFilters.searchTerm); | ||
| }, [tableIdentifier]); |
There was a problem hiding this comment.
getFilters is referenced inside this effect but omitted from the dependency array, which violates the exhaustive-deps rule and can lead to stale data being loaded if sharedFilters changes after mount.
| }, [tableIdentifier]); | |
| }, [tableIdentifier, getFilters]); |
|
@livebitz1 Thanks for your work! But I prefer the other ones—they’re shorter and simpler |
What does this PR do?
This PR implements filter persistence across booking tabs to improve user experience. When users switch between different booking tabs (upcoming, unconfirmed, recurring, past, cancelled), their filter preferences are now maintained.
Visual Demo
Before:
After:
Mandatory Tasks
How should this be tested?
Set up:
Test Steps:
Expected Behavior:
Checklist
Summary by cubic
Filters now stay applied when switching between booking tabs and after refreshing the page, making it easier to manage bookings.