[Refactor] Profile page | Implement the new design for visited sites tab #3582
Conversation
|
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/date.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/apps.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. apps/web/lib/features/activity/components/visited-Item.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 makes several minor formatting and implementation adjustments in the date helper functions and updates the visited site components. Changes include updating function signatures and return values, renaming components (e.g., from AppVisitedItem to VisitedItem), and reworking the VisitedSitesTab layout and data handling. New localization keys for visited sites are also added across multiple language files. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as VisitedSitesTab
participant H as useTimeDailyActivity
participant VI as VisitedItem
participant VS as VisitedItemSkeleton
U->>V: Request visited sites
V->>H: Fetch sites with parameter "SITE"
H-->>V: Return visited sites (or empty array)
alt Sites available
V->>VI: Render each visited site
else No sites
V->>VS: Render skeleton loading state
end
Possibly related issues
Possibly related PRs
Suggested labels
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 (
|
![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 (2)
apps/web/app/helpers/date.ts (2)
72-79: Return undefined instead of 0 for invalid dates.The function documentation states it should return
undefinedfor invalid dates, but it returns0. This should be fixed to match the documentation.export function differenceBetweenHours(startedAt: Date, stoppedAt: Date): number { const started = new Date(startedAt); const stopped = new Date(stoppedAt); if (!isNaN(started.getTime()) && !isNaN(stopped.getTime())) { return (stopped.getTime() - started.getTime()) / 1000; } - return 0; + return undefined; }🧰 Tools
🪛 Biome (1.9.4)
[error] 75-75: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 75-75: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
92-99: Add error handling for date parsing.The
formatTimeFromDatefunction should handle invalid date strings gracefully.export const formatTimeFromDate = (date: string | Date | undefined) => { if (!date) return ''; const dateObject = date instanceof Date ? date : new Date(date); + if (isNaN(dateObject.getTime())) { + return ''; + } const hours = dateObject.getHours().toString().padStart(2, '0'); const minutes = dateObject.getMinutes().toString().padStart(2, '0'); return `${hours}:${minutes}`; };
🧹 Nitpick comments (4)
apps/web/lib/features/activity/components/visited-Item.tsx (2)
52-55: Consider extracting time formatting logic to a utility function.The time formatting logic could be moved to a separate utility function to improve reusability and maintainability.
+// In a new file: utils/formatTime.ts +export const formatTimeDisplay = (h: number, m: number, s: number): string => { + return `${String(h).padStart(2, '0')}:${String(m).padStart(2, '0')}:${String(s).padStart(2, '0')}`; +}; // In visited-Item.tsx -<p style={{ flexBasis: itemCellsWidth['time-spent-in-hours'] }} className=""> - {`${String(h).padStart(2, '0')}:${String(m).padStart(2, '0')}:${String(s).padStart(2, '0')}`} -</p> +<p style={{ flexBasis: itemCellsWidth['time-spent-in-hours'] }} className=""> + {formatTimeDisplay(h, m, s)} +</p>
19-27: Consider moving itemCellsWidth outside component.The
itemCellsWidthobject could be defined outside the component since it doesn't depend on any props or state. This would prevent unnecessary recreations on each render.+const ITEM_CELLS_WIDTH = { + apps: '20%', + 'visited-dates': '25%', + 'percent-used': '40%', + 'time-spent-in-hours': '15%' +} as const; const VisitedItem = ({ app, totalMilliseconds, type }: Props) => { - const itemCellsWidth = useMemo( - () => ({ - apps: '20%', - 'visited-dates': '25%', - 'percent-used': '40%', - 'time-spent-in-hours': '15%' - }), - [] - ); + const itemCellsWidth = ITEM_CELLS_WIDTH;apps/web/lib/features/activity/visited-sites.tsx (2)
14-34: Consider moving headers outside component.The headers array could be defined outside the component and memoized with only the translation function as a dependency.
+const getHeaders = (t: TranslationHooks) => [ + { + title: t('timer.VISITED_SITES'), + width: '20%' + }, + { + title: t('timer.VISITED_DATES'), + width: '25%' + }, + { + title: t('timer.PERCENT_USED'), + width: '40%' + }, + { + title: t('timer.TIME_SPENT_IN_HOURS'), + width: '15%' + } +]; export function VisitedSitesTab() { const { visitedSites, loading } = useTimeDailyActivity('SITE'); const t = useTranslations(); const sites = groupAppsByHour(visitedSites ?? []); - const headers = useMemo( - () => [ - { - title: t('timer.VISITED_SITES'), - width: '20%' - }, - // ... other headers - ], - [t] - ); + const headers = useMemo(() => getHeaders(t), [t]);
71-76: Loading state could be more specific.The loading state check could be simplified and made more specific to avoid potential race conditions.
-{loading && visitedSites?.length < 1 && ( +{loading && ( <> <VisitedItemSkeleton /> <VisitedItemSkeleton /> </> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/app/helpers/date.ts(5 hunks)apps/web/lib/features/activity/apps.tsx(3 hunks)apps/web/lib/features/activity/components/visited-Item.tsx(2 hunks)apps/web/lib/features/activity/components/visited-item-skeleton.tsx(1 hunks)apps/web/lib/features/activity/visited-sites.tsx(1 hunks)apps/web/locales/ar.json(1 hunks)apps/web/locales/bg.json(1 hunks)apps/web/locales/de.json(1 hunks)apps/web/locales/en.json(1 hunks)apps/web/locales/es.json(1 hunks)apps/web/locales/fr.json(1 hunks)apps/web/locales/he.json(1 hunks)apps/web/locales/it.json(1 hunks)apps/web/locales/nl.json(1 hunks)apps/web/locales/pl.json(1 hunks)apps/web/locales/pt.json(1 hunks)apps/web/locales/ru.json(1 hunks)apps/web/locales/zh.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/lib/features/activity/components/visited-item-skeleton.tsx
- apps/web/lib/features/activity/apps.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (14)
apps/web/lib/features/activity/components/visited-Item.tsx (1)
7-15: LGTM! Component props are well-typed and documented.The interface definition is clear and the props are properly typed with meaningful names.
apps/web/locales/zh.json (1)
716-717: LGTM! Translations are properly added.The new translations for visited sites are consistent with the feature requirements and other language files.
apps/web/locales/he.json (1)
716-717: Ensure Proper Localization in Hebrew:
The new keys"VISITED_SITES": "אתרים שביקרת בהם"and"NO_VISITED_SITE_MESSAGE": "אין אתרים שביקרת בהם"have been added in the "timer" section as intended for the visited sites feature. The translations appear accurate and consistent in tone with other keys in the file.apps/web/locales/ar.json (1)
716-717: Verify Arabic Translation Consistency:
The additions"VISITED_SITES": "المواقع التي تمت زيارتها"and"NO_VISITED_SITE_MESSAGE": "لا يوجد مواقع تمت زيارتها"in the "timer" section are correctly implemented and provide necessary labels for displaying visited sites. Please ensure that these translations align with your overall Arabic localization standards and the new design’s terminology.apps/web/locales/en.json (1)
717-718: Confirm English Labels for Visited Sites:
The keys"VISITED_SITES": "Visited sites"and"NO_VISITED_SITE_MESSAGE": "There are no visited sites"have been added in the "timer" section, clearly reflecting the new functionality. Ensure that these labels are integrated correctly within the UI components for the visited sites tab.apps/web/locales/nl.json (1)
716-717: Dutch Localization for Visited Sites AddedThe new keys
"VISITED_SITES": "Bezochte sites"and"NO_VISITED_SITE_MESSAGE": "Er zijn geen bezochte sites"have been added to the timer section. They appear to be correctly localized for Dutch. Please ensure that the corresponding application code uses these keys consistently for rendering the visited sites tab.apps/web/locales/bg.json (1)
715-717: Bulgarian Localization for Visited Sites AddedThe keys
"VISITED_SITES": "Посетени сайтове"and"NO_VISITED_SITE_MESSAGE": "Няма посетени сайтове"have been included. The translations accurately reflect the intended messages for Bulgarian users. Confirm that these keys are integrated with the visited sites component in the UI.apps/web/locales/pl.json (1)
716-718: Polish Localization for Visited Sites AddedThe additions of
"VISITED_SITES": "Odwiedzone strony"and"NO_VISITED_SITE_MESSAGE": "Brak odwiedzonych stron"are consistent with similar changes in other languages. Please verify that the new keys are referenced properly within the visited sites tab implementation.apps/web/locales/it.json (1)
715-717: New Localization Keys Added for Visited SitesThe addition of
"VISITED_SITES": "Siti visitati"and"NO_VISITED_SITE_MESSAGE": "Non ci sono siti visitati"is clear and consistent with the new design requirements for the visited sites tab. Ensure these values are aligned with the UI design and match the terminology used in other language files.apps/web/locales/ru.json (1)
715-717: New Localization Keys Added for Visited SitesThe new keys
"VISITED_SITES": "Посещенные сайты"and"NO_VISITED_SITE_MESSAGE": "Нет посещенных сайтов"have been added correctly in the "timer" section. They are consistent with the overall localization strategy for the visited sites tab.apps/web/locales/pt.json (1)
715-717: New Localization Keys Added for Visited SitesThe additions of
"VISITED_SITES": "Sites visitados"and"NO_VISITED_SITE_MESSAGE": "Não há sites visitados"appropriately extend the timer’s localization. Their inclusion provides the necessary text for indicating visited sites and handling empty states.apps/web/locales/es.json (1)
716-717: New Localization Keys Added for Visited SitesThe keys
"VISITED_SITES": "Sitios visitados"and"NO_VISITED_SITE_MESSAGE": "No hay sitios visitados"have been added within the"timer"section. They follow the expected naming conventions and are consistent with similar entries in other locale files. Please ensure that these translations align with design and product terminology across the application.apps/web/locales/de.json (1)
716-717: New Localization Entries Added for Visited SitesThe additions
"VISITED_SITES": "Besuchte Seiten"and"NO_VISITED_SITE_MESSAGE": "Es gibt keine besuchten Seiten"in the"timer"object correctly extend the localization support. The German translations are accurate and consistent with similar updates in other files. It is recommended to verify that the capitalization and phrasing match any existing style guidelines for your project.apps/web/locales/fr.json (1)
715-716: Addition of Visited Sites Localization KeysThe new keys
"VISITED_SITES": "Sites visités"and"NO_VISITED_SITE_MESSAGE": "Il n'y a pas de sites visités"have been integrated into the"timer"section. These entries maintain consistency with the multilingual update across the project and appear correct. Please double-check that the terminology aligns with the overall product language guidelines.
evereq
deleted the
3579-improvement-profile-page-implement-the-new-design-for-the-visited-site-tab
branch
Description
Closes #3579
Type of Change
Checklist
Current screenshots
Summary by CodeRabbit