[Feat]: Functionalize TeamDashboardFilter component with optimized state management #3576
Conversation
|
Caution Review failedThe pull request is closed. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces several changes across the codebase. Two new words have been added in the spell-check configuration file. The team dashboard filtering functionality has been refactored by extracting filter logic into a new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DH as DashboardHeader
participant TF as TeamDashboardFilter
participant H as useTimelogFilterOptions
participant S as State Store
U->>DH: Clicks filter button
DH->>TF: Loads new TeamDashboardFilter
TF->>U: Displays popover with multi-select options
U->>TF: Selects teams/employees and applies filter
TF->>H: Triggers update with new filter selections
H->>S: Updates alluser/allteams state atoms
S-->>H: Returns updated state
H-->>TF: Confirms filter update
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx (1)
9-12: Remove unused proponUpdateFilters.The
onUpdateFiltersprop is no longer used after moving the filter functionality to theTeamDashboardFiltercomponent.Apply this diff to remove the unused prop:
interface DashboardHeaderProps { onUpdateDateRange: (startDate: Date, endDate: Date) => void; - onUpdateFilters: (filters: Partial<Omit<ITimeLogReportDailyChartProps, 'organizationId' | 'tenantId'>>) => void; } -export function DashboardHeader({ onUpdateDateRange, onUpdateFilters }: DashboardHeaderProps) { +export function DashboardHeader({ onUpdateDateRange }: DashboardHeaderProps) {
🧹 Nitpick comments (5)
apps/web/app/stores/time-logs.ts (1)
28-29: Fix inconsistent spacing around equals signs.The spacing around equals signs in the new atoms is inconsistent with the rest of the file.
Apply this diff to maintain consistent spacing:
-export const allTeamsState=atom<IOrganizationTeamList[]>([]); -export const allUserState=atom<OT_Member[]>([]); +export const allTeamsState = atom<IOrganizationTeamList[]>([]); +export const allUserState = atom<OT_Member[]>([]);apps/web/lib/features/team/teams-dropdown.tsx (1)
28-37: Simplify the timer status condition.The condition checking timer status has multiple redundant checks and could be simplified for better readability.
Apply this diff to simplify the condition:
if ( timerStatus && timerStatus?.running && - timerStatus?.lastLog && - timerStatus?.lastLog?.organizationTeamId && - timerStatus?.lastLog?.source === 'TEAMS' && activeTeam && activeTeam?.id && - timerStatus?.lastLog?.organizationTeamId === activeTeam?.id + timerStatus?.lastLog?.source === 'TEAMS' && + timerStatus?.lastLog?.organizationTeamId === activeTeam.id ) {🧰 Tools
🪛 Biome (1.9.4)
[error] 29-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/hooks/features/useTimelogFilterOptions.ts (2)
9-10: Fix missing semicolons.Maintain consistent semicolon usage throughout the file.
Apply this diff:
- const [alluserState, setAllUserState] = useAtom(allUserState) - const [allteamsState, setAllTeamsState] = useAtom(allTeamsState) + const [alluserState, setAllUserState] = useAtom(allUserState); + const [allteamsState, setAllTeamsState] = useAtom(allTeamsState);
18-18: Remove commented code.Remove the commented-out code as it's no longer needed and can cause confusion.
Apply this diff:
- // const [selectTimesheetId, setSelectTimesheetId] = React.useState<TimesheetLog[]>([])apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx (1)
70-101: Improve type safety in MultiSelect components.The type casting with 'as any' in onValueChange handlers should be replaced with proper types.
Consider defining proper types and updating the handlers:
interface Team { id: string; name: string; members?: Member[]; } interface Member { id: string; employee?: { fullName: string; }; } // Update handlers onValueChange={(selectedItems: Team[]) => setAllTeamsState(selectedItems)} onValueChange={(selectedItems: Member[]) => setAllUserState(selectedItems)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.cspell.json(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx(2 hunks)apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx(2 hunks)apps/web/app/hooks/features/useTimelogFilterOptions.ts(2 hunks)apps/web/app/stores/time-logs.ts(2 hunks)apps/web/lib/features/team/teams-dropdown.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (3)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx (2)
1-14: LGTM! Well-structured imports and component declaration.The component is properly optimized with React.memo and imports all necessary dependencies.
30-50: LGTM! Well-implemented filter button with accessibility support.The button implementation includes proper accessibility attributes and responsive styling.
.cspell.json (1)
17-18: LGTM! Spell-check dictionary updated.The new words "alluser" and "allteams" are correctly added to support the state management variables.
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx
Outdated
Show resolved
Hide resolved
|
@Innocent-Akim merge conflict :) |
Innocent-Akim
force-pushed
the
feat/team-dashboard-filter-functionalization
branch
from
e571ca6 to
67cb3e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx (1)
10-11: UnusedonUpdateFiltersProp in DashboardHeaderProps
Our investigation confirms that whileonUpdateFiltersis declared in theDashboardHeaderPropsinterface and passed from the parent (page.tsx), it isn’t used anywhere within theDashboardHeadercomponent or forwarded to theTeamDashboardFiltercomponent. This indicates that the prop is likely redundant since filter handling has been moved toTeamDashboardFilter.
• RemoveonUpdateFiltersfrom the interface (and corresponding props in parent components) if it’s no longer needed, or
• Pass the callback toTeamDashboardFilterif it is intended to handle filter updates.🔗 Analysis chain
Consider updating interface to reflect new filter handling.
The
onUpdateFiltersprop is still in the interface but appears unused after moving filter logic toTeamDashboardFiltercomponent.Consider either:
- Removing unused props and their types if filtering is now fully handled by
TeamDashboardFilter- Passing the callback to
TeamDashboardFilterif it's still neededinterface DashboardHeaderProps { onUpdateDateRange: (startDate: Date, endDate: Date) => void; - onUpdateFilters: (filters: Partial<Omit<ITimeLogReportDailyChartProps, 'organizationId' | 'tenantId'>>) => void; }🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Searching for occurrences of "onUpdateFilters" across the repository to verify its usage. rg -n "onUpdateFilters" .Length of output: 531
Script:
#!/bin/bash # Displaying the contents of dashboard-header.tsx to see how onUpdateFilters is used. sed -n '1,150p' apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsxLength of output: 1731
🧹 Nitpick comments (3)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx (3)
15-20: Simplify state reset logic.The current implementation using
useEffectto resetshouldRemoveItemscan be simplified.Consider using a callback instead:
- const [shouldRemoveItems, setShouldRemoveItems] = React.useState(false); - React.useEffect(() => { - if (shouldRemoveItems) { - setShouldRemoveItems(false); - } - }, [shouldRemoveItems]); + const handleClearFilter = React.useCallback(() => { + setShouldRemoveItems(true); + // Reset immediately after MultiSelect processes it + setTimeout(() => setShouldRemoveItems(false), 0); + }, []);
32-32: Add aria-label to Popover for better accessibility.The Popover component lacks an accessible label.
- <Popover modal> + <Popover modal aria-label={t('common.FILTER_OPTIONS')}>
104-115: Add keyboard navigation support for filter actions.The filter actions section lacks keyboard navigation support.
Add keyboard support for better accessibility:
- <div className="flex gap-x-4 justify-end items-center w-full"> + <div + className="flex gap-x-4 justify-end items-center w-full" + role="group" + aria-label={t('common.FILTER_ACTIONS')} + > <Button onClick={() => setShouldRemoveItems(true)} variant={'outline'} className="flex justify-center items-center h-10 text-sm rounded-lg dark:text-gray-300" + onKeyDown={(e) => e.key === 'Enter' && setShouldRemoveItems(true)} > <span className="text-sm">{t('common.CLEAR_FILTER')}</span> </Button> <Button className="flex justify-center items-center h-10 text-sm rounded-lg bg-primary dark:bg-primary-light dark:text-gray-300" + onKeyDown={(e) => e.key === 'Enter' && handleApplyFilter()} > <span className="text-sm">{t('common.APPLY_FILTER')}</span> </Button> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.cspell.json(1 hunks)apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx(2 hunks)apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx(2 hunks)apps/web/app/hooks/features/useTimelogFilterOptions.ts(2 hunks)apps/web/app/stores/time-logs.ts(2 hunks)apps/web/lib/features/team/teams-dropdown.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .cspell.json
- apps/web/app/[locale]/timesheet/[memberId]/components/TimeSheetFilterPopover.tsx
- apps/web/lib/features/team/teams-dropdown.tsx
- apps/web/app/stores/time-logs.ts
- apps/web/app/hooks/features/useTimelogFilterOptions.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (1)
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/dashboard-header.tsx (1)
27-27: Pass onUpdateFilters to TeamDashboardFilter if still needed.The
onUpdateFiltersprop is not being passed toTeamDashboardFilter, which may indicate a loss of functionality.If filtering updates should still be propagated to the parent:
- <TeamDashboardFilter /> + <TeamDashboardFilter onUpdateFilters={onUpdateFilters} />
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/dashboard/team-dashboard/[teamId]/components/team-dashboard-filter.tsx
Outdated
Show resolved
Hide resolved
Description
Please include a summary of the changes and the related issues.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of the previous status
Current screenshots
Please add here videos or images of the current (new) status
Summary by CodeRabbit