Repository navigation
docs: api url, feat: files, feat: collapsible and feat: limit - #82
Conversation
- updated the production API url across the project, especially in the dashboard and the vscode extension - released a new version (`0.0.19`) of the extension with the new api - updated the refs to the API url in the READMEs - moved `PeriodProjects.tsx` to `dashboard-page`
- installed the shadcn tooltip component - replaced the raw title when hovering the files by a tooltip - displayed the total time spent on the files of a language in the FilesList
- made each language in the files list collapsible
- added a limit parameter to allow the user to pick a set of files - added a controlled input bound to the limit to take the input from the user - extracted the filters (group and limit) in their own separated component - corrected the GetProjectFilesOnPeriodDto to make the amount lte 0
WalkthroughThis update introduces enhancements to the dashboard's file listing UI, including new sorting and limiting controls, collapsible language groups, and tooltips for file paths. Supporting hooks and components were refactored or added to enable these features. Documentation and configuration files were updated to reflect new API endpoints, dependency additions, and version increments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardUI
participant FiltersSection
participant Files
participant useFiles (hook)
participant API
User->>DashboardUI: Loads project page
DashboardUI->>FiltersSection: Render filter controls (limit, group, sort)
User->>FiltersSection: Adjusts filters (limit, group, sort)
FiltersSection->>Files: Pass updated props (amount, isGrouped, isSortedDesc)
Files->>useFiles (hook): Fetch files with filters
useFiles (hook)->>API: Request files with amount, sorting params
API-->>useFiles (hook): Return files data
useFiles (hook)-->>Files: Provide grouped/sorted files
Files-->>DashboardUI: Render files with tooltips, collapsible groups
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 3
🧹 Nitpick comments (12)
apps/vscode-extension/README.md (1)
11-12: Version badge correctly bumped – consider automating the sync.The new
0.0.19badge is in line with thepackage.jsonversion.
To avoid future drifts, you might add a small release script (or GitHub Action) that replaces the badge string automatically whenever the extension version is bumped.apps/vscode-extension/package.json (1)
5-5: LGTM – extension version updated.No other metadata changes detected. Remember to update/append the CHANGELOG for 0.0.19 if you keep one.
README.md (1)
24-25: Minor inconsistencies & typos in updated prose.
- Line 24 – “vscode extension” should be capitalised for consistency with other bullets (“VSCode extension”).
- Line 32 – the new API URL looks good.
- Lines 34-35 – wording “served by the extension” is clear, but double-check that the port note still matches the default configured in the extension (I believe it’s 4208 → OK).
- Several occurrences of “detailled” → “detailed”.
These are small, but tightening them up keeps the README polished.
Also applies to: 32-35
apps/api/README.md (1)
83-90: Docker commands – tag & run look correct, small clarity tweak.Good move placing
-tbefore-f. For readability you could rename the image/tag to include the version (mooncode-api:0.0.1) and then reference the same tag in thedocker runexample.No functional issues.
apps/dashboard/src/hooks/projects/useFiles.tsx (1)
61-80: Consider extracting the data transformation logic for better readability.The data transformation logic is correct but quite complex. The tuple creation with explicit typing and total time calculation works well, but consider extracting this into a separate utility function for improved maintainability.
Example refactor:
const createLanguageGroups = (groups: [string, FileType[]][]) => { return groups.map(([languageSlug, files]) => [ { languageSlug, totalTimeSpentOnLanguage: files.reduce( (acc, file) => acc + file.totalTimeSpent, 0, ), }, files, ] as [LanguageMetadata, FileType[]]); };apps/dashboard/src/components/project-page/files-list/FilesList.tsx (1)
22-25: Consider input validation and user feedback.The limit input lacks validation feedback to users. Consider adding visual indicators for invalid input states or restricting input to numbers only.
You could enhance the user experience by:
- Adding input type="number" restriction
- Showing validation messages for invalid inputs
- Adding min/max constraints
apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx (3)
12-13: Remove unnecessary ESLint disable comment.The
handleLimitInputChangeparameter is actually used in the component (line 22), so the ESLint disable comment is unnecessary and misleading.- // eslint-disable-next-line no-unused-vars handleLimitInputChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
4-15: Consider extracting props interface for better maintainability.The inline props interface works but extracting it would improve readability and reusability.
+interface FiltersSectionProps { + limitInput: string; + isGrouped: boolean; + handleLimitInputChange: (e: React.ChangeEvent<HTMLInputElement>) => void; + handleCheckChange: () => void; +} + -const FiltersSection = ({ - limitInput, - isGrouped, - handleLimitInputChange, - handleCheckChange, -}: { - limitInput: string; - isGrouped: boolean; - handleLimitInputChange: (e: React.ChangeEvent<HTMLInputElement>) => void; - handleCheckChange: () => void; -}) => ( +const FiltersSection = ({ + limitInput, + isGrouped, + handleLimitInputChange, + handleCheckChange, +}: FiltersSectionProps) => (
18-23: Consider adding input constraints for better UX.The limit input could benefit from additional attributes to guide user input.
<Input value={limitInput} aria-label="limit" placeholder="Enter a limit" + type="number" + min="1" onChange={handleLimitInputChange} />apps/dashboard/src/components/project-page/files-list/Files.tsx (2)
27-38: Optimize collapsed languages state management.Using array operations for tracking collapsed state can be inefficient with many languages. Consider using a Set or Record for better performance.
- const [collapsedLanguages, setCollapsedLanguages] = useState<string[]>([]); + const [collapsedLanguages, setCollapsedLanguages] = useState<Set<string>>(new Set()); - const isLanguageCollapsed = (languageSlug: string) => - collapsedLanguages.includes(languageSlug); + const isLanguageCollapsed = (languageSlug: string) => + collapsedLanguages.has(languageSlug); const handleCollapseButtonClick = (languageSlug: string) => { - setCollapsedLanguages( - isLanguageCollapsed(languageSlug) - ? collapsedLanguages.filter((entry) => entry !== languageSlug) - : [...collapsedLanguages, languageSlug], - ); + setCollapsedLanguages(prev => { + const newSet = new Set(prev); + if (newSet.has(languageSlug)) { + newSet.delete(languageSlug); + } else { + newSet.add(languageSlug); + } + return newSet; + }); };
76-94: Consider performance implications of multiple TooltipProvider instances.Having TooltipProvider wrap each file list might create unnecessary context providers. Consider moving TooltipProvider to a higher level or using a single provider for the entire component.
return ( + <TooltipProvider> <ul className={cn("flex flex-col", isGrouped && "space-y-16")}> {isGrouped ? ( groups.map( ([{ languageSlug, totalTimeSpentOnLanguage }, groupedFiles]) => ( <ul> {/* ... */} {!isLanguageCollapsed(languageSlug) && ( - <TooltipProvider> {groupedFiles.map(/* ... */)} - </TooltipProvider> )} </ul> ), ) ) : ( - <TooltipProvider> {files.map(/* ... */)} - </TooltipProvider> )} </ul> + </TooltipProvider> );Also applies to: 100-118
packages/ui/src/components/ui/tooltip.tsx (1)
21-21: Consider breaking down the long className for maintainability.The className string is quite long and could benefit from being split across multiple lines or organized into logical groups for better readability.
className={cn( - "z-50 overflow-hidden rounded-md bg-neutral-900 px-3 py-1.5 text-xs text-neutral-50 animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-tooltip-content-transform-origin] dark:bg-neutral-50 dark:text-neutral-900", + [ + "z-50 overflow-hidden rounded-md bg-neutral-900 px-3 py-1.5 text-xs text-neutral-50", + "animate-in fade-in-0 zoom-in-95", + "data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95", + "data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2", + "data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", + "origin-[--radix-tooltip-content-transform-origin]", + "dark:bg-neutral-50 dark:text-neutral-900" + ].join(" "), className, )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
README.md(1 hunks)apps/api/README.md(4 hunks)apps/api/src/files-stats/files-stats.dto.ts(1 hunks)apps/dashboard/src/components/dashboard-page/Dashboard.tsx(1 hunks)apps/dashboard/src/components/dashboard-page/PeriodProjects.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/Files.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesList.tsx(3 hunks)apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx(1 hunks)apps/dashboard/src/hooks/projects/useFiles.tsx(3 hunks)apps/vscode-extension/README.md(1 hunks)apps/vscode-extension/package.json(1 hunks)apps/vscode-extension/src/constants.ts(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/ui/tooltip.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/dashboard/src/components/project-page/files-list/FilesList.tsx (1)
apps/dashboard/src/types-schemas.ts (1)
Entry(46-50)
apps/dashboard/src/hooks/projects/useFiles.tsx (1)
apps/dashboard/src/constants.ts (1)
PERIODS_CONFIG(78-121)
packages/ui/src/components/ui/tooltip.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (12)
apps/api/README.md (1)
7-8: Public URL updated – verify CORS and TLS settings.Changing to
api.mooncode.ccis fine; ensure the API server’s CORS config and TLS certificates are already issued for the new domain to prevent the extension or dashboard from failing pre-flight requests.packages/ui/package.json (1)
36-36: LGTM! Dependency addition supports tooltip feature.The addition of
@radix-ui/react-tooltipat version^1.2.7correctly supports the new tooltip functionality introduced in this PR.apps/api/src/files-stats/files-stats.dto.ts (1)
51-51: LGTM! Validation change appropriately allows zero values.Changing from
positive()tononnegative()correctly allows zero as a valid amount value, which makes sense for a limit parameter where zero could represent "no limit" or "show all files".apps/dashboard/src/components/dashboard-page/Dashboard.tsx (1)
8-8: Import Path Verified:PeriodProjects.tsxExists at New LocationThe
PeriodProjectscomponent file was found atapps/dashboard/src/components/dashboard-page/PeriodProjects.tsx, confirming the updated import path is correct. No further action is needed—this change can be approved as is.apps/dashboard/src/components/dashboard-page/PeriodProjects.tsx (2)
47-47: LGTM! Grid layout simplification improves maintainability.The change from auto-fit grid with minmax to a simpler fixed 2-column grid with explicit gaps is more predictable and easier to maintain while preserving responsive behavior.
53-53: LGTM! Icon styling consistency improvement.Switching from
stroke-*totext-*classes for icon coloring is more consistent with Tailwind best practices and provides better control over icon appearance.Also applies to: 57-57
apps/dashboard/src/hooks/projects/useFiles.tsx (2)
8-11: LGTM! Hook signature update correctly adds amount parameter.The addition of the optional
amountparameter properly supports the new file limiting functionality.
25-25: LGTM! Amount parameter correctly passed to query options.The
amountparameter is properly included in both custom range and predefined period query options.Also applies to: 32-32
apps/dashboard/src/components/project-page/files-list/FilesList.tsx (1)
1-1: Well-structured component refactoring.Good separation of concerns by extracting the filters into a dedicated
FiltersSectioncomponent. The props are properly typed and passed down correctly.Also applies to: 6-6, 60-65
apps/dashboard/src/components/project-page/files-list/Files.tsx (2)
64-74: Good accessibility implementation for collapsible UI.The collapse functionality includes proper ARIA labels and descriptive titles, which enhances accessibility for screen readers.
48-50: Smart conditional styling for collapsed states.The conditional border and padding application based on collapse state provides a clean visual transition and good UX.
packages/ui/src/components/ui/tooltip.tsx (1)
12-27: Excellent tooltip implementation with comprehensive styling.The TooltipContent component demonstrates best practices:
- Proper forwardRef usage for ref forwarding
- Comprehensive animation states and directional slide effects
- Dark mode support
- Portal usage for proper layering
- Configurable sideOffset with sensible default
- added filters to sort the filters in asc/desc order both when they are a list or grouped. The groups are sorted externally and also internally
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/dashboard/src/hooks/projects/files/useFiles.tsx (1)
83-88: LGTM: Proper conditional sorting with immutable approach.The sorting logic correctly uses the spread operator for immutable array reversal. However, the parameter name
isSortedDescmight be slightly confusing sincetruereturns original order whilefalsereverses it, implying the original data is already in descending order.Consider adding a comment to clarify the expected data order or the sorting behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/src/components/project-page/files-list/Files.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesList.tsx(3 hunks)apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx(1 hunks)apps/dashboard/src/hooks/projects/files/useFiles.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/src/components/project-page/files-list/FilesList.tsx
- apps/dashboard/src/components/project-page/files-list/Files.tsx
- apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx
🔇 Additional comments (3)
apps/dashboard/src/hooks/projects/files/useFiles.tsx (3)
8-12: LGTM: Function signature properly expanded for new features.The addition of
amountandisSortedDescparameters aligns well with the PR objectives for limit and sorting functionality. The types are appropriate and parameter naming is clear.
26-26: LGTM: Consistent parameter addition across query branches.The
amountparameter is correctly added to both query branches, maintaining consistency in the API call structure.Also applies to: 33-33
62-81: LGTM: Well-structured data transformation with proper typing.The transformation logic correctly groups files by language and calculates aggregate totals. The explicit tuple typing ensures proper TypeScript inference, and the comment demonstrates good awareness of type system requirements.
Note that this adds computational complexity with the nested reduce operation, but this is likely acceptable for typical file counts.
- added validation to prevent negative values of the limit in the `FilesList` component - added a comment to clarify that the data returned by the server is in initially in DESC order (`useFiles` hook)
- refactor the header of groups in a FilesGroupHeader component - improved the ui for the files group
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx (5)
21-24: Remove unnecessary ESLint disable comments.The ESLint disable comments for
no-unused-varsare unnecessary since the function parameters are actually used in the component. These comments add noise to the code.- // eslint-disable-next-line no-unused-vars handleSortButtonClick: (languageSlug: string) => void; - // eslint-disable-next-line no-unused-vars handleCollapseButtonClick: (languageSlug: string) => void;
46-47: Improve accessibility labels for better UX.The aria-label and title should be more descriptive and user-friendly. Consider using language-specific labels that mention the actual language being sorted.
- aria-label={`Sort files ${isLanguageAscSorted ? "descending" : "ascending"}`} - title={`Click to sort ${isLanguageAscSorted ? "descending" : "ascending"}`} + aria-label={`Sort ${getLanguageName(languageSlug)} files ${isLanguageAscSorted ? "descending" : "ascending"}`} + title={`Click to sort ${getLanguageName(languageSlug)} files ${isLanguageAscSorted ? "descending" : "ascending"}`}
21-24: Consider removing unnecessary eslint-disable comments.The
eslint-disablecomments forno-unused-varsappear unnecessary since the handler functions do use their parameters when called (lines 45 and 54). These comments might be misleading.- // eslint-disable-next-line no-unused-vars handleSortButtonClick: (languageSlug: string) => void; - // eslint-disable-next-line no-unused-vars handleCollapseButtonClick: (languageSlug: string) => void;
21-24: Remove unnecessary eslint-disable comments.The
handleSortButtonClickandhandleCollapseButtonClickfunctions are used in the component (lines 45 and 54), so the eslint-disable comments for unused variables are unnecessary.- // eslint-disable-next-line no-unused-vars handleSortButtonClick: (languageSlug: string) => void; - // eslint-disable-next-line no-unused-vars handleCollapseButtonClick: (languageSlug: string) => void;
55-56: Improve accessibility label for collapse button.The aria-label should be more descriptive and consistent with the title attribute.
- aria-label="collapse language" - title={!isLanguageCollapsed ? "collapse" : "extend"} + aria-label={`${isLanguageCollapsed ? "Expand" : "Collapse"} ${getLanguageName(languageSlug)} files`} + title={`${isLanguageCollapsed ? "Expand" : "Collapse"} ${getLanguageName(languageSlug)} files`}apps/dashboard/src/components/project-page/files-list/Files.tsx (6)
57-59: Optimize array operations for better performance.Creating a new array with spread operator and then reversing it on every render is inefficient. Consider memoizing the sorted result.
+ const sortedGroupedFiles = useMemo(() => + isLanguageAscSorted ? [...groupedFiles].reverse() : groupedFiles, + [groupedFiles, isLanguageAscSorted] + ); - const sortedGroupedFiles = isLanguageAscSorted - ? [...groupedFiles].reverse() - : groupedFiles;Don't forget to import
useMemofrom React:-import { useState } from "react"; +import { useState, useMemo } from "react";
80-100: Consider TooltipProvider placement optimization.Having TooltipProvider inside the conditional render and wrapping all tooltips is correct, but consider if multiple providers might cause issues if this component is used multiple times on the same page.
Consider moving TooltipProvider to a higher level in the component tree if multiple Files components might be rendered simultaneously to avoid potential conflicts.
27-36: Optimize state management with Set for better performance.Using arrays for
collapsedLanguagesand frequentincludes()calls can be inefficient. Consider using aSetfor O(1) lookups instead of O(n).- const [collapsedLanguages, setCollapsedLanguages] = useState<string[]>([]); - const getIsLanguageCollapsed = (languageSlug: string) => - collapsedLanguages.includes(languageSlug); - const handleCollapseButtonClick = (languageSlug: string) => { - setCollapsedLanguages( - getIsLanguageCollapsed(languageSlug) - ? collapsedLanguages.filter((entry) => entry !== languageSlug) - : [...collapsedLanguages, languageSlug], - ); - }; + const [collapsedLanguages, setCollapsedLanguages] = useState<Set<string>>(new Set()); + const getIsLanguageCollapsed = (languageSlug: string) => + collapsedLanguages.has(languageSlug); + const handleCollapseButtonClick = (languageSlug: string) => { + setCollapsedLanguages(prev => { + const newSet = new Set(prev); + if (newSet.has(languageSlug)) { + newSet.delete(languageSlug); + } else { + newSet.add(languageSlug); + } + return newSet; + }); + };
38-47: Apply the same Set optimization to ascSortedLanguages.Similar to the collapsed languages, use a
Setfor better performance with the ascending sorted languages state.- const [ascSortedLanguages, setAscSortedLanguages] = useState<string[]>([]); - const getIsLanguageAscSorted = (languageSlug: string) => - ascSortedLanguages.includes(languageSlug); - const handleSortButtonClick = (languageSlug: string) => { - setAscSortedLanguages( - getIsLanguageAscSorted(languageSlug) - ? ascSortedLanguages.filter((entry) => entry !== languageSlug) - : [...ascSortedLanguages, languageSlug], - ); - }; + const [ascSortedLanguages, setAscSortedLanguages] = useState<Set<string>>(new Set()); + const getIsLanguageAscSorted = (languageSlug: string) => + ascSortedLanguages.has(languageSlug); + const handleSortButtonClick = (languageSlug: string) => { + setAscSortedLanguages(prev => { + const newSet = new Set(prev); + if (newSet.has(languageSlug)) { + newSet.delete(languageSlug); + } else { + newSet.add(languageSlug); + } + return newSet; + }); + };
57-59: Optimize array reversal to avoid unnecessary re-creation.The array reversal creates a new array on every render. Consider memoizing this operation or using a more efficient approach.
- const sortedGroupedFiles = isLanguageAscSorted - ? [...groupedFiles].reverse() - : groupedFiles; + const sortedGroupedFiles = useMemo(() => + isLanguageAscSorted ? [...groupedFiles].reverse() : groupedFiles, + [groupedFiles, isLanguageAscSorted] + );Don't forget to import
useMemofrom React:-import { useState } from "react"; +import { useState, useMemo } from "react";
80-100: Consider moving TooltipProvider higher in the component tree.Having multiple
TooltipProviderinstances might cause unnecessary re-renders. Consider moving it to wrap the entire component or higher up in the component tree.return ( + <TooltipProvider> - <ul className={cn("flex w-full flex-col", isGrouped && "gap-y-4")}> + <ul className={cn("flex w-full flex-col", isGrouped && "gap-y-4")}> {isGrouped ? ( groups.map( // ... existing code ... - {!isLanguageCollapsed && ( - <TooltipProvider> {sortedGroupedFiles.map( // ... tooltip content ... )} - </TooltipProvider> - )} // ... rest of grouped mode ) : ( - <TooltipProvider> {files.map(([filePath, { name, totalTimeSpent }]) => ( // ... tooltip content ... ))} - </TooltipProvider> )} - </ul> + </ul> + </TooltipProvider> );Also applies to: 108-126
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/src/components/project-page/files-list/Files.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesList.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/components/project-page/files-list/FilesList.tsx
👮 Files not reviewed due to content moderation or server errors (2)
- apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx
- apps/dashboard/src/components/project-page/files-list/Files.tsx
🔇 Additional comments (15)
apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx (6)
1-5: LGTM! Clean import organization.The imports are well-organized with external dependencies first, followed by internal utilities. All imports appear to be used in the component.
1-6: LGTM!The imports are well-organized and all necessary for the component functionality. Good use of specific icon imports and utility functions from the monorepo packages.
26-39: LGTM!The UI structure is well-organized with proper semantic HTML and Tailwind classes. The use of inline styles for dynamic language colors is appropriate, and the layout clearly separates the language information from the controls.
41-58: LGTM!The controls section is well-implemented with proper accessibility attributes. The aria-labels correctly describe the action that will be performed when clicked, and the conditional rendering for the collapse icon is appropriate.
7-61: Well-structured component with good accessibility features.The component follows React best practices with proper TypeScript typing, accessibility attributes, and clean separation of concerns. The use of semantic HTML and proper event handling makes it maintainable.
30-32: No XSS risk:languageColoris sourced from an internal, static mappingThe
getLanguageColorfunction returns values from a predefinedlanguagesAttributesmap (falling back to a constantDEFAULT_COLOR). Since neither the color map nor its keys (languageSlug) come from user input, there’s no injection surface in this inline style. You can safely disregard the XSS concern here.Likely an incorrect or invalid review comment.
apps/dashboard/src/components/project-page/files-list/Files.tsx (9)
1-7: LGTM! Proper tooltip component imports.The tooltip components are correctly imported and will be used throughout the component for enhanced UX.
27-36: LGTM! Clean state management for collapsed languages.The state management logic for collapsed languages is well-implemented with clear helper functions and proper state updates.
38-47: LGTM! Consistent state management pattern.The ascending sort state management follows the same clean pattern as the collapsed languages implementation.
25-25: useFiles parameters verified – no changes neededThe hook signature in useFiles.tsx
(languagesToFetch: string[] | undefined, amount: number | undefined, isSortedDesc: boolean)
matches the call in Files.tsx. No action is required.
1-24: LGTM!The new imports for tooltip functionality and the updated component interface with
amountandisSortedDescprops properly support the enhanced file listing features described in the PR objectives.
25-47: LGTM!The state management for collapsed languages and ascending sort order is well-implemented. The helper functions provide clean APIs, and the toggle logic properly maintains immutability by creating new arrays. The consistent pattern between collapse and sort functionality makes the code maintainable.
50-106: LGTM!The grouped mode rendering is well-implemented with proper integration of the FilesGroupHeader component, conditional styling based on collapse state, and tooltip functionality. The sorting logic correctly reverses the array when ascending sort is selected, assuming the base order from the hook is descending.
107-127: LGTM!The ungrouped mode rendering is clean and straightforward. It properly implements tooltip functionality for file paths and maintains consistency with the grouped mode styling. The simpler structure is appropriate for the ungrouped display.
1-132: Excellent enhancement with interactive features and accessibility.The component successfully integrates tooltips, collapsible groups, and sorting functionality while maintaining good code organization and accessibility. The new features significantly improve the user experience for file navigation.
| <div className="inline-block min-h-9 space-x-4"> | ||
| <span className="min-h-9 w-full overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | ||
| • {name} | ||
| </span> | ||
| <span className="whitespace-nowrap font-extralight"> | ||
| {formatDuration(totalTimeSpent)} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix layout issues with inline-block and spacing.
The current layout using inline-block and space-x-4 may cause inconsistent spacing. Consider using flexbox for better control.
- <div className="inline-block min-h-9 space-x-4">
- <span className="min-h-9 w-full overflow-hidden text-ellipsis whitespace-nowrap font-extrabold">
+ <div className="flex min-h-9 items-center justify-between gap-4">
+ <span className="flex-1 overflow-hidden text-ellipsis whitespace-nowrap font-extrabold">
• {name}
</span>
<span className="whitespace-nowrap font-extralight">
{formatDuration(totalTimeSpent)}
</span>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="inline-block min-h-9 space-x-4"> | |
| <span className="min-h-9 w-full overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | |
| • {name} | |
| </span> | |
| <span className="whitespace-nowrap font-extralight"> | |
| {formatDuration(totalTimeSpent)} | |
| </span> | |
| </div> | |
| <div className="flex min-h-9 items-center justify-between gap-4"> | |
| <span className="flex-1 overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | |
| • {name} | |
| </span> | |
| <span className="whitespace-nowrap font-extralight"> | |
| {formatDuration(totalTimeSpent)} | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In apps/dashboard/src/components/project-page/files-list/Files.tsx around lines
86 to 93, replace the div with class "inline-block min-h-9 space-x-4" by a flex
container to improve layout consistency. Change the div to use "flex" and
"space-x-4" classes to enable flexbox layout and maintain spacing between child
spans. This will provide better control over alignment and spacing compared to
inline-block.
| <div className="inline-block min-h-9 space-x-4"> | ||
| <span className="overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | ||
| • {name} | ||
| </span> | ||
| <span className="whitespace-nowrap font-extralight"> | ||
| {formatDuration(totalTimeSpent)} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent layout fix to ungrouped mode.
The same layout issue exists in ungrouped mode. Apply the same flexbox solution for consistency.
- <div className="inline-block min-h-9 space-x-4">
- <span className="overflow-hidden text-ellipsis whitespace-nowrap font-extrabold">
+ <div className="flex min-h-9 items-center justify-between gap-4">
+ <span className="flex-1 overflow-hidden text-ellipsis whitespace-nowrap font-extrabold">
• {name}
</span>
<span className="whitespace-nowrap font-extralight">
{formatDuration(totalTimeSpent)}
</span>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="inline-block min-h-9 space-x-4"> | |
| <span className="overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | |
| • {name} | |
| </span> | |
| <span className="whitespace-nowrap font-extralight"> | |
| {formatDuration(totalTimeSpent)} | |
| </span> | |
| </div> | |
| <div className="flex min-h-9 items-center justify-between gap-4"> | |
| <span className="flex-1 overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> | |
| • {name} | |
| </span> | |
| <span className="whitespace-nowrap font-extralight"> | |
| {formatDuration(totalTimeSpent)} | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In apps/dashboard/src/components/project-page/files-list/Files.tsx around lines
113 to 120, the layout for the ungrouped mode does not use the flexbox styling
applied elsewhere, causing inconsistency. Update the container div wrapping the
name and duration spans to use the same flexbox classes (e.g., "flex
items-center space-x-4") instead of "inline-block min-h-9 space-x-4" to ensure
consistent alignment and spacing.
- made the files section properly responsive on small screens
- added a search functionality to allow the user to look for a particular file - made the search and limit input debounced to limit the number of rerenders and to optimize for performance - memoized the Files component for a better performance and to make the input filling fluid
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/dashboard/src/components/project-page/files-list/FilesList.tsx (1)
36-48: Input validation logic correctly implemented.The input validation has been properly improved to handle edge cases, validate positive integers, and set undefined for invalid inputs. This addresses the previous review feedback effectively.
apps/dashboard/src/components/project-page/files-list/Files.tsx (2)
101-108: Layout issues resolved with proper flexbox implementation.The layout now uses flexbox (
flex min-h-9 gap-4) instead of the problematic inline-block approach, providing consistent spacing and alignment.
128-135: Consistent flexbox layout applied to ungrouped mode.The ungrouped mode now uses the same flexbox layout as the grouped mode, ensuring consistent styling throughout the component.
🧹 Nitpick comments (3)
apps/dashboard/src/hooks/projects/files/useFiles.tsx (2)
39-41: Consider server-side filtering for better performance.The client-side filtering works correctly but could become inefficient with large datasets. Consider moving the search filtering to the server-side query if performance becomes an issue.
43-84: Consider extracting complex grouping logic into a separate utility function.The grouping and mapping logic is quite complex and makes the hook harder to read and test. Consider extracting this into a separate utility function.
+const groupFilesByLanguage = (files: typeof files) => { + const groupedFiles = files.reduce( + (acc, [filePath, file]) => { + const { languageSlug } = file; + if (!acc[languageSlug]) { + acc[languageSlug] = []; + } + acc[languageSlug].push({ ...file, filePath }); + return acc; + }, + {} as { [languageSlug: string]: any[] } + ); + + return Object.entries(groupedFiles).map( + ([languageSlug, rest]) => [ + { + languageSlug, + totalTimeSpentOnLanguage: rest.reduce( + (acc, value) => acc + value.totalTimeSpent, + 0 + ), + }, + rest, + ] as const + ); +}; - const groups = Object.entries( - files.reduce( - // ... complex logic - ), - ).map(/* ... */); + const groups = groupFilesByLanguage(files);apps/dashboard/src/components/project-page/files-list/Files.tsx (1)
95-117: Consider extracting file item rendering into a separate component.The file item rendering logic with tooltips is repeated in both grouped and ungrouped modes. Consider extracting this into a reusable
FileItemcomponent to reduce duplication and improve maintainability.+const FileItem = ({ filePath, name, totalTimeSpent }: { + filePath: string; + name: string; + totalTimeSpent: number; +}) => ( + <li key={filePath}> + <Tooltip> + <TooltipTrigger asChild> + <div className="flex min-h-9 gap-4"> + <span className="min-h-9 overflow-hidden text-ellipsis whitespace-nowrap font-extrabold"> + • {name} + </span> + <span className="overflow-hidden text-ellipsis whitespace-nowrap font-extralight"> + {formatDuration(totalTimeSpent)} + </span> + </div> + </TooltipTrigger> + <TooltipContent>{filePath}</TooltipContent> + </Tooltip> + </li> +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/dashboard/src/components/project-page/files-list/Files.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx(1 hunks)apps/dashboard/src/components/project-page/files-list/FilesList.tsx(2 hunks)apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx(1 hunks)apps/dashboard/src/hooks/projects/files/useFiles.tsx(3 hunks)apps/dashboard/src/hooks/useDebounce.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/src/components/project-page/files-list/FilesGroupHeader.tsx
- apps/dashboard/src/components/project-page/files-list/FiltersSection.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/src/components/project-page/files-list/FilesList.tsx (2)
apps/dashboard/src/types-schemas.ts (1)
Entry(46-50)apps/dashboard/src/hooks/useDebounce.tsx (1)
useDebounce(3-17)
🔇 Additional comments (5)
apps/dashboard/src/hooks/useDebounce.tsx (1)
1-17: LGTM! Clean and well-implemented debounce hook.The implementation follows React best practices with proper cleanup, generic typing, and correct dependency management. This will effectively optimize performance by reducing unnecessary API calls from rapid user inputs.
apps/dashboard/src/hooks/projects/files/useFiles.tsx (1)
85-91: LGTM! Sorting logic is clean and handles immutability correctly.The conditional sorting with array spreading ensures immutability while providing the expected sort behavior.
apps/dashboard/src/components/project-page/files-list/FilesList.tsx (2)
19-34: Well-organized state management with proper debouncing.The state variables and handlers are clearly organized, and the debouncing implementation will effectively reduce API calls and improve performance.
72-80: Good separation of concerns with FiltersSection component.Extracting the filter controls into a separate component improves modularity and maintainability.
apps/dashboard/src/components/project-page/files-list/Files.tsx (1)
14-26: Good use of memoization for performance optimization.Memoizing the component prevents unnecessary re-renders when parent components update, which is especially important given the complex rendering logic.
Commits
docs: api url
0.0.19) of the extension with the new apiPeriodProjects.tsxtodashboard-pagefeat: files
feat: collapsible
feat: limit
GetProjectFilesOnPeriodDtoto make the amount lte 0Summary by CodeRabbit
New Features
Improvements
Documentation
Dependency Updates
@radix-ui/react-tooltipto UI package dependencies.Bug Fixes