[Refactor] Profile page | visited apps tab UI #3578
Conversation
CREDO23
added
enhancement
|
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/helpers/array-data.tsOops! 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. apps/web/app/interfaces/timer/ITimerApp.tsOops! 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. apps/web/lib/features/activity/screenshoots.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.
WalkthroughThe PR introduces several code modifications across UI helper functions, interfaces, and components. In the helper, a new check for an application’s duration is added before grouping apps by hour. The Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GroupHelper as groupAppsByHour
Caller->>GroupHelper: Pass app object
alt app.duration is defined
GroupHelper->>GroupHelper: Check and update/create hour entry
GroupHelper-->>Caller: Return updated grouped data
else app.duration is undefined
GroupHelper-->>Caller: Skip processing
end
sequenceDiagram
participant Component as AppVisitedItem
participant AppData as App Object
Component->>AppData: Retrieve app data
alt app.duration exists
Component->>Component: Compute time components and percent value
else
Component->>Component: Default values {h:0, m:0, s:0} with undefined percent
end
Component->>ProgressBar: Render with backgroundColor "black"
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/web/lib/features/activity/components/app-visited-Item.tsx (1)
16-17: Improve number formatting for better readability.The time and percentage calculations handle optional fields correctly, but the number formatting could be improved.
Consider using Intl.NumberFormat for consistent number formatting:
-const percent = app?.duration && ((+app.duration * 100) / totalMilliseconds).toFixed(2); +const percent = app?.duration + ? new Intl.NumberFormat('en-US', { + maximumFractionDigits: 2 + }).format((+app.duration * 100) / totalMilliseconds) + : undefined;apps/web/lib/features/activity/apps.tsx (1)
64-69: Simplify loading state condition.The loading state check could be simplified.
Consider this simpler condition:
-{loading && visitedApps?.length < 1 && ( +{loading && (apps/web/lib/components/progress-bar.tsx (1)
20-33: Simplify color logic with a utility function.The color logic could be extracted into a utility function for better maintainability.
Consider this refactor:
+const getProgressColor = (progress: number) => { + if (progress < 25) return 'bg-red-600 dark:bg-red-400'; + if (progress < 50) return 'bg-yellow-600 dark:bg-yellow-400'; + if (progress < 75) return 'bg-blue-600 dark:bg-blue-400'; + return 'bg-green-600 dark:bg-green-400'; +}; <div className={clsxm( ' h-2 rounded-full absolute z-[1] bg-black ', - +parseInt(progress.toString()) < 25 && 'bg-red-600 dark:bg-red-400', - +parseInt(progress.toString()) >= 25 && - +parseInt(progress.toString()) < 50 && - 'bg-yellow-600 dark:bg-yellow-400', - +parseInt(progress.toString()) >= 50 && - +parseInt(progress.toString()) < 75 && - 'bg-blue-600 dark:bg-blue-400', - +parseInt(progress.toString()) >= 75 && 'bg-green-600 dark:bg-green-400' + getProgressColor(+parseInt(progress.toString())) )} style={{ width: progress }} />apps/web/lib/features/activity/screenshoots.tsx (1)
26-26: Consider memoizing the formatted activity percentage.The UI improvement to consistently show percentage format is good. However, since this calculation is done on every render, consider memoizing it for better performance.
+ const formattedActivityPercent = useMemo(() => { + return Number.isNaN(parseFloat(activityPercent.toFixed(2))) ? '0%' : activityPercent.toFixed(2); + }, [activityPercent]); <h2 className="text-3xl font-medium"> - {Number.isNaN(parseFloat(activityPercent.toFixed(2))) ? '0%' : activityPercent.toFixed(2)} + {formattedActivityPercent} </h2>apps/web/app/helpers/array-data.ts (1)
37-46: Improve code structure and readability.While the defensive programming approach is good, the code structure could be improved by:
- Flattening the if-else nesting
- Avoiding repeated time slice operation
- Making type conversion more explicit
- if (app.duration) - if (hourDataIndex !== -1) { - groupedData[hourDataIndex].apps.push(app); - groupedData[hourDataIndex].totalMilliseconds += +app.duration; - } else - groupedData.push({ - hour: app.time.slice(0, 5), - totalMilliseconds: +app.duration, - apps: [app] - }); + if (!app.duration) { + return; + } + + const hour = app.time.slice(0, 5); + const duration = Number(app.duration); + + if (hourDataIndex !== -1) { + groupedData[hourDataIndex].apps.push(app); + groupedData[hourDataIndex].totalMilliseconds += duration; + return; + } + + groupedData.push({ + hour, + totalMilliseconds: duration, + apps: [app] + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/server-web/release/app/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
apps/web/app/helpers/array-data.ts(1 hunks)apps/web/app/interfaces/timer/ITimerApp.ts(1 hunks)apps/web/lib/components/progress-bar.tsx(2 hunks)apps/web/lib/features/activity/apps.tsx(1 hunks)apps/web/lib/features/activity/components/app-visited-Item.tsx(2 hunks)apps/web/lib/features/activity/screenshoots.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
- GitHub Check: deploy
🔇 Additional comments (2)
apps/web/lib/features/activity/components/app-visited-Item.tsx (1)
19-27: LGTM! Good performance optimization.The use of useMemo for layout calculations prevents unnecessary recalculations.
apps/web/lib/features/activity/apps.tsx (1)
14-34: LGTM! Excellent optimization with proper dependencies.The headers configuration is properly memoized with the translation function as a dependency.
CREDO23
force-pushed
the
3398-refactor-profile-page--update-visited-apps-tab-ui
branch
from
442f30e to
8b446e4
Compare
CREDO23
force-pushed
the
3398-refactor-profile-page--update-visited-apps-tab-ui
branch
from
8b446e4 to
3d8cded
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/app/interfaces/timer/ITimerApp.ts (2)
11-12: Document the purpose and usage of new fields.The new fields
durationPercentageandchildItemslack documentation explaining their purpose and expected values.Consider adding JSDoc comments to document:
- The meaning of
durationPercentage(e.g., is it a value between 0-1 or 0-100?)- The relationship between parent and child items
- Any constraints on the values
export interface ITimerApps { time: string; sessions?: number; duration?: number; employeeId?: string; date?: string; title?: string; description?: string; + /** Percentage of time spent on this app (0-100) */ durationPercentage?: number; + /** Nested timer apps, e.g., for grouping related activities */ childItems?: ITimerApps[]; }
8-8: Consider using a more specific date type.The
datefield is typed as an optional string, which provides no type safety for date format validation.Consider using a more specific type or adding validation:
- Use a union type with specific date format strings
- Add runtime validation for date strings
- Consider using a Date object if date manipulation is needed
- date?: string; + /** Date in ISO 8601 format (YYYY-MM-DD) */ + date?: `${number}-${number}-${number}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/helpers/array-data.ts(1 hunks)apps/web/app/interfaces/timer/ITimerApp.ts(1 hunks)apps/web/lib/components/progress-bar.tsx(2 hunks)apps/web/lib/features/activity/apps.tsx(1 hunks)apps/web/lib/features/activity/components/app-visited-Item.tsx(2 hunks)apps/web/lib/features/activity/screenshoots.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/lib/components/progress-bar.tsx
- apps/web/lib/features/activity/screenshoots.tsx
- apps/web/lib/features/activity/apps.tsx
- apps/web/lib/features/activity/components/app-visited-Item.tsx
- apps/web/app/helpers/array-data.ts
🔇 Additional comments (2)
apps/web/app/interfaces/timer/ITimerApp.ts (2)
4-12: Ensure proper handling of optional fields across components.The interface changes make several critical fields optional, which could lead to runtime issues if not handled properly in components.
Let's verify the handling of these optional fields in components:
#!/bin/bash # Description: Search for direct property access without optional chaining # Focus on components that might not handle undefined values properly # Search for direct property access without optional chaining or nullish checks rg -g '*.{ts,tsx}' --type-add 'tsx:*.{ts,tsx}' -A 2 'const.*=.*app\.(sessions|duration|employeeId|date|title|description|durationPercentage|childItems)[^?]' # Search for array operations on childItems that might not handle undefined rg -g '*.{ts,tsx}' --type-add 'tsx:*.{ts,tsx}' -A 2 'app\.childItems\.(map|filter|reduce|forEach|some|every)'
5-6: Consider type consistency for numeric fields.The
sessionsanddurationfields are now typed as optional numbers instead of strings. While this is a more appropriate type, ensure consistent type usage:
- Update any string-to-number conversions in components
- Verify that API responses match these numeric types
Let's check for any remaining string-to-number conversions:
✅ Verification successful
Numeric Field Consistency Verified
The search for string-to-number conversions (usingparseInt,parseFloat, and string methods) on thesessionsanddurationfields yielded no matches. This indicates that the codebase consistently treats these fields as numbers, and no unintended string operations are being performed on them.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find potential type mismatches in numeric field handling # Search for parseInt/parseFloat usage on these fields rg -g '*.{ts,tsx}' --type-add 'tsx:*.{ts,tsx}' -A 2 '(parseInt|parseFloat)\(.*\.(sessions|duration)' # Search for string operations on these numeric fields rg -g '*.{ts,tsx}' --type-add 'tsx:*.{ts,tsx}' -A 2 '\.(sessions|duration)\.(toString|split|replace)'Length of output: 206
evereq
deleted the
3398-refactor-profile-page--update-visited-apps-tab-ui
branch
Description
Closes #3398
Type of Change
Checklist
Current screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Refactor