Mariano/cleanup - #133
Mariano/cleanup#133
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request removes obsolete navigation links in dashboard home components and deprecates multiple legacy evidence filtering and listing components. A new structure is introduced under the evidence/list directory that includes updated versions of the EvidenceList, UI states, summary cards, and table components with enhanced navigation. Import paths and export indices have been adjusted accordingly without altering the core functionality of the evidence tasks display. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant EL as EvidenceList Component
participant API as Evidence Data Hook/API
participant UI as Evidence UI States
U->>EL: Load evidence list page
EL->>API: Request evidence tasks & stats
alt Data is loading
EL->>UI: Render Skeleton state
else Error occurs
EL->>UI: Render Error state with retry option
else Data available
EL->>UI: Render SummaryCards, SearchInput, FilterDropdown, and EvidenceListTable
U->>EL: Click on a table row
EL->>U: Navigate to detailed evidence view
end
Possibly related PRs
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/SearchInput.tsx (2)
17-28: Consider optimizing the effect dependency arrayThe useEffect dependency array includes both setSearch and setPage, which are likely stable function references from the context and don't need to be included in the dependency array.
useEffect(() => { if (debouncedValue === "") { setSearch(null); } else { setSearch(debouncedValue); } setPage("1"); // Reset to first page when searching - }, [debouncedValue, setSearch, setPage]); + }, [debouncedValue]);
33-39: Add aria-label for improved accessibilityThe search input would benefit from an explicit aria-label for screen reader users.
<Input type="search" placeholder={placeholder} + aria-label="Search evidence" className="w-full pl-8 h-10" value={inputValue} onChange={(e) => setInputValue(e.target.value)} />apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/ActiveFilterBadges.tsx (2)
33-96: Extract repeated badge pattern to reduce duplicationThere's significant repetition in the badge rendering pattern. Consider extracting this to a helper function or component to improve maintainability.
+ function FilterBadge({ + label, + value, + onClear + }: { + label: string; + value: string; + onClear: () => void + }) { + return ( + <Badge variant="outline" className="flex items-center gap-1"> + {label}: {value} + <X + className="h-3 w-3 cursor-pointer" + onClick={onClear} + /> + </Badge> + ); + } export function ActiveFilterBadges() { // existing code... return ( <div className="flex flex-wrap gap-2 mt-2"> {status && ( - <Badge variant="outline" className="flex items-center gap-1"> - Status: {status} - <X - className="h-3 w-3 cursor-pointer" - onClick={() => { - setStatus(null); - setPage("1"); - }} - /> - </Badge> + <FilterBadge + label="Status" + value={status} + onClear={() => { + setStatus(null); + setPage("1"); + }} + /> )} {/* Apply similar changes to other badges */} </div> ); }
41-43: Consider creating a reusable filter reset functionEach filter has the same pattern for resetting - set the value to null and reset to page 1. This could be extracted into a helper function.
export function ActiveFilterBadges() { const { // ... existing destructuring } = useEvidenceTable(); + const resetFilter = (setFilter: (value: null) => void) => { + setFilter(null); + setPage("1"); + }; // ... rest of the component // Example usage: <X className="h-3 w-3 cursor-pointer" - onClick={() => { - setStatus(null); - setPage("1"); - }} + onClick={() => resetFilter(setStatus)} /> }Also applies to: 53-55, 65-67, 77-79, 89-91
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListHeader.tsx (2)
22-31: Consider extracting column visibility logic to a constants fileThe column visibility logic (columns hidden on mobile) is hardcoded. Consider extracting these column IDs to a constants file for better maintainability.
// Create a new file: constants.ts + export const COLUMNS_HIDDEN_ON_MOBILE = [ + "status", + "department", + "frequency", + "nextReviewDate", + "assignee", + "relevance" + ]; // In EvidenceListHeader.tsx + import { COLUMNS_HIDDEN_ON_MOBILE } from "../../constants"; // Then in the component <TableHead key={header.id} className={cn( "p-4 relative whitespace-nowrap", - (header.id === "status" || - header.id === "department" || - header.id === "frequency" || - header.id === "nextReviewDate" || - header.id === "assignee" || - header.id === "relevance") && - "hidden md:table-cell", + COLUMNS_HIDDEN_ON_MOBILE.includes(header.id) && "hidden md:table-cell", )} style={{ width: header.getSize() }} >
56-65: Extract resizing handler into a separate componentThe resizing handler code is complex and could be extracted to a separate component for better readability.
+ function ColumnResizer({ header, table }) { + return ( + <div + onMouseDown={header.getResizeHandler()} + onTouchStart={header.getResizeHandler()} + className={`absolute right-0 top-0 h-full w-1 cursor-col-resize select-none touch-none bg-border opacity-0 hover:opacity-100 ${ + table.getState().columnSizingInfo.isResizingColumn === + header.column.id + ? "bg-primary opacity-100" + : "" + }`} + /> + ); + } // Then in the main component - <div - onMouseDown={header.getResizeHandler()} - onTouchStart={header.getResizeHandler()} - className={`absolute right-0 top-0 h-full w-1 cursor-col-resize select-none touch-none bg-border opacity-0 hover:opacity-100 ${ - table.getState().columnSizingInfo.isResizingColumn === - header.column.id - ? "bg-primary opacity-100" - : "" - }`} - /> + <ColumnResizer header={header} table={table} />apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/PaginationControls.tsx (2)
12-12: Consider using absolute imports instead of deeply nested relative paths.The import path
../../../hooks/useEvidenceTableContextuses multiple levels of parent directory navigation, which can become brittle if files are moved or restructured. Consider using absolute imports or path aliases configured in your tsconfig to make imports more maintainable.
33-34: Remove duplicate CSS classes.There are duplicate
flex items-center space-x-2classes on adjacent divs. Consider removing the redundant styling by restructuring these elements.<div className="flex items-center space-x-2"> - <div className="flex items-center space-x-2"> + <div> <p className="text-sm font-medium">Rows per page</p> <Select value={pageSize} onValueChange={handlePageSizeChange}>apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/FilterDropdown.tsx (1)
60-60: Consider responsive design for the dropdown widthThe dropdown has a fixed width of 500px with a max-width constraint of 90vw. While this works for most cases, consider testing on various screen sizes to ensure optimal display.
- <DropdownMenuContent align="end" className="w-[500px] max-w-[90vw]"> + <DropdownMenuContent align="end" className="w-[500px] max-w-[90vw] md:max-h-[80vh] overflow-y-auto">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/app/src/app/[locale]/(app)/(dashboard)/(home)/components/FrameworkProgress.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/(home)/components/RequirementStatusChart.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/ActiveFilterBadges.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/FilterDropdown.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/PaginationControls.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/SearchInput.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceList.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceSummaryCards.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/data-table/index.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/index.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceList.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceListUIStates.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceSummaryCards.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/ActiveFilterBadges.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/FilterDropdown.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/PaginationControls.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/SearchInput.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListColumns.tsx(8 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListHeader.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListTable.tsx(5 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/hooks/useEvidenceTableContext.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/page.tsx(1 hunks)
💤 Files with no reviewable changes (10)
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/data-table/index.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/(home)/components/FrameworkProgress.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/PaginationControls.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/(home)/components/RequirementStatusChart.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/SearchInput.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceSummaryCards.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/ActiveFilterBadges.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/index.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceFilters/FilterDropdown.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/components/EvidenceList.tsx
🔇 Additional comments (41)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/SearchInput.tsx (1)
1-42: Well-structured search input component with debounce implementationThis search component is properly implemented with debounce functionality to prevent excessive API calls. The component:
- Uses the useDebounce hook with a reasonable 500ms delay
- Correctly handles empty searches by setting search to null
- Appropriately resets pagination when search criteria changes
- Has a clean UI with a search icon and proper styling
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/ActiveFilterBadges.tsx (2)
23-26: Nice handling of assignee name retrievalGood implementation for finding the assignee name from the assignees array. The fallback to "Unknown" is a good practice.
28-31: Efficient conditional renderingThe component efficiently returns null when no filters are active, avoiding unnecessary rendering.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListHeader.tsx (1)
6-6: Good renaming to improve code clarityThe renaming from
DataTableHeadertoEvidenceListHeaderand related interface improves the naming consistency with the domain. The updated import path also maintains code organization.Also applies to: 10-10, 14-14
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/page.tsx (2)
1-2: Import paths updated correctlyThe import paths have been appropriately updated to reflect the new component structure, which is in line with the PR's goal of reorganizing the codebase.
4-10: Clean and concise page componentThe page component is well-structured with a clear separation of concerns. It properly wraps the EvidenceList with the necessary context provider.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/PaginationControls.tsx (1)
14-78: Well-structured pagination component with good UX considerations.The PaginationControls component is well-implemented with:
- Appropriate disabling of navigation buttons when on first/last page
- Reset to first page when changing page size
- Clear display of current position (x of y pages)
- Good error handling (returning null when pagination data is unavailable)
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/index.ts (2)
1-3: Clean and well-structured index exports.This index file effectively centralizes exports from the table components, making them easily importable by other parts of the application. The structure follows common patterns for React applications.
1-1:Details
❓ Verification inconclusive
Verify component naming consistency.
The export uses the name
DataTablewhile importing from a file namedEvidenceListTable. This suggests the component might be renamed during export. Ensure this is intentional and consistent with how the component is used throughout the application.
🏁 Script executed:
#!/bin/bash # Check for the DataTable component definition in EvidenceListTable.tsx echo "Checking component naming in EvidenceListTable.tsx:" fd "EvidenceListTable.tsx" --exec grep -l "export.*(DataTable|as DataTable)" {} \;Length of output: 198
Action needed: Verify Component Export Naming Convention
The index file re-exports the component asDataTablefrom"./EvidenceListTable", but our automated check didn’t locate an export (or alias) namedDataTablewithin theEvidenceListTable.tsxfile. Please confirm whether the renaming is intentional. If so, assure that:
- The exported component from
EvidenceListTable.tsxis meant to be referenced asDataTablethroughout the application.- All usages of this component in the codebase correctly match the re-exported name.
If this renaming is unintended or causes inconsistency compared with the original definition, consider aligning the export name with the component’s native name.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/index.ts (1)
1-3: Well-organized component exports.This index file provides a clean API for importing evidence list components from a single location. Using the wildcard export for filters is appropriate as they are likely all related components that should be exported together.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceList.tsx (4)
26-32: Great user experience handling for loading states.Excellent approach tracking both evidence tasks and statistics loading states. Showing a skeleton loading state when either is loading ensures a consistent user experience without partial content rendering.
34-36: Good error handling with retry functionality.The error state includes a retry mechanism through the
mutatefunction, which provides users with a way to recover from temporary failures without refreshing the entire page.
48-58: Well-structured conditional rendering.The component elegantly handles the empty state with appropriate filtering controls, while also ensuring the table and pagination only render when there's actual data to display.
15-61: Well-designed component architecture using context and hooks.The EvidenceList component follows best practices by:
- Separating concerns through hooks for data and state management
- Using appropriate context providers for shared state
- Handling all possible UI states (loading, error, empty, populated)
- Maintaining clear component boundaries with good composition
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceSummaryCards.tsx (3)
1-11: Well-structured component with appropriate imports and client-side directiveThe component correctly uses the "use client" directive and imports necessary UI components and icons. The import path for the hook is relative, which is appropriate for accessing hooks within the same module structure.
8-35: Excellent handling of loading and error statesThe component implements proper loading and error states with appropriate visual feedback. The skeleton loaders match the structure of the actual content, providing a smooth loading experience.
37-108: Well-designed summary cards with clear visual indicatorsThe grid layout with status cards provides a clean, organized view of evidence task statistics. Each card effectively communicates its purpose with appropriate icons, colors, and descriptive text.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListColumns.tsx (4)
3-10: Reorganized imports for better readabilityThe imports have been reorganized in a more logical manner, grouping related components together.
11-11: Renamed export to better reflect its purposeRenaming from
columnstoEvidenceListColumnsimproves code clarity by making the export name more descriptive and specific to its function.
22-22: Simplified name column renderingThe name column rendering has been simplified by removing Button and Link components, replacing them with a simple span element. This might affect navigation capability if users previously could click on the name to navigate.
Was this change intentional? If users need to navigate to task details by clicking on the name, you might need to restore some form of interactive element.
36-36: Disabled sorting for multiple columnsSorting has been disabled for several columns that previously had it enabled. This might impact user experience if sorting was a frequently used feature.
Is there a specific reason for disabling sorting on these columns? If this was intentional as part of a larger architecture change, disregard this comment. Otherwise, consider whether users would benefit from being able to sort by these fields.
Also applies to: 56-56, 83-83, 98-98, 130-130, 165-165
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/EvidenceListUIStates.tsx (3)
1-34: Well-implemented loading state with skeletonsThe
EvidenceListSkeletoncomponent provides excellent visual feedback during loading states. The skeleton placeholders match the structure of the actual content, creating a smooth loading experience.
36-66: Comprehensive error handling with retry functionalityThe
EvidenceListErrorcomponent effectively communicates errors to users with a clear message and optional retry functionality. The component properly handles cases where an error message might not be available.
68-110: Well-designed empty state with contextual informationThe
EvidenceListEmptycomponent intelligently adapts its display based on whether filters are applied, providing contextual information to help users understand why no results are shown.apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceFilters/FilterDropdown.tsx (4)
1-42: Well-structured component with appropriate context usageThe component imports necessary UI elements and uses the
useEvidenceTablehook to access filtering state and functions. The destructuring of context values is clean and organized.
43-59: Effective filter button with active filter countThe filter button provides good visual feedback by displaying a badge with the count of active filters, making it clear to users when filters are applied.
60-189: Well-organized filter options with two-column layoutThe filter options are effectively organized into two columns, grouping related filters together for better usability. Each filter section has clear labels and appropriate icons.
191-203: Helpful clear filters button when filters are activeThe component provides a "Clear all filters" button when filters are applied, making it easy for users to reset their filter selections.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/components/table/EvidenceListTable.tsx (12)
12-14: Updated import paths reflect new structureThe imports have been successfully updated to match the new folder structure, maintaining good separation of concerns between columns, header, and types.
17-17: Added router for navigationGood addition of the Next.js router for handling navigation to evidence detail pages.
19-20: Component renamed for better clarityThe component has been renamed from a generic
DataTableto the more specificEvidenceListTablewhich better describes its purpose.
30-30: Using renamed column definitionProperly updated to use the renamed
EvidenceListColumnsconstant.
35-38: Added default column sizingGood addition of default column sizing properties for better table layout consistency.
41-44: Custom size for relevance columnAppropriate custom sizing for the relevance column improves table display by preventing this column from taking too much space.
49-51: Added row click navigation handlerGood implementation of row click navigation that enhances user experience by making the entire row clickable for viewing evidence details.
60-60: Updated header component referenceCorrectly updated to use the renamed
EvidenceListHeadercomponent.
67-68: Added clickable row styling and event handlingGood UX enhancement with cursor-pointer styling and click handler for navigation.
82-82: Updated column stylingProperly maintained styling for the relevance column to ensure consistent display.
99-102: Prevented event propagation during column resizeGood addition of event propagation stopping to prevent navigation when resizing columns, which would otherwise create a confusing user experience.
111-111: Updated empty state colspanCorrectly updated the colspan to use the length of the new columns array.
apps/app/src/app/[locale]/(app)/(dashboard)/evidence/list/hooks/useEvidenceTableContext.tsx (1)
6-9: Updated import paths for restructured componentsImport paths have been properly updated to reflect the new directory structure, maintaining correct references to hooks, constants, and types after the restructuring.
Stop storing employeeName and employeeEmail in sessionStorage during the Stripe billing redirect flow. Only requesterNotes (non-PII) is persisted. After redirect, employeeName re-derives from the employee prop and the email field resets to its default. Resolves code-scanning alert trycompai#133 (clear text storage of sensitive information). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# [3.39.0](v3.38.1...v3.39.0) (2026-04-30) ### Bug Fixes * **api:** correct fileData max-length to 100MB and allow localhost in URL validation ([6d56a2d](6d56a2d)) * **api:** correct type casting for rawBody in main.ts ([7e624d4](7e624d4)) * **app:** remove product app marketing pixels ([#2716](#2716)) ([26d75be](26d75be)) * **background-checks:** fix 13 bugs across billing, webhooks, custom uploads, and UI ([d5df5db](d5df5db)) * **background-checks:** remove employee PII from sessionStorage ([ea082b3](ea082b3)), closes [#133](#133) * **background-checks:** security hardening across payment flow, validation, and logging ([8b3b39b](8b3b39b)) * **pentest:** bump split-view breakpoint from md to xl for tablet support ([efd1c85](efd1c85)) ### Features * **background-checks:** add employee background checks ([0456df6](0456df6))
Summary by CodeRabbit
New Features
Refactor