[WEB-4323] refactor: Analytics refactor #7213
Conversation
…lytics-minor-improvements
… y-axis label to use new translation
…ese locales in translations.json files
…te analytics tab import to use getAnalyticsTabs function
…lytics-minor-improvements
WalkthroughThe updates rename the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnalyticsPage
participant getAnalyticsTabs
participant TabsComponent
User->>AnalyticsPage: Visit analytics page
AnalyticsPage->>getAnalyticsTabs: Call getAnalyticsTabs(t)
getAnalyticsTabs-->>AnalyticsPage: Return localized tab configs
AnalyticsPage->>TabsComponent: Render Tabs with tab configs
TabsComponent-->>User: Display analytics tabs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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: 1
🧹 Nitpick comments (3)
packages/constants/src/analytics/common.ts (1)
14-14: Addreadonly/as constfor safer immutability & better type-narrowing
ANALYTICS_INSIGHTS_FIELDSis a pure lookup table that shouldn’t be mutated at runtime. Marking the structure as immutable gives you:
- Compile-time prevention of accidental writes.
- Precise string-literal inference when consumers index into the record.
-export const ANALYTICS_INSIGHTS_FIELDS: Record<TAnalyticsTabsBase, IInsightField[]> = { +export const ANALYTICS_INSIGHTS_FIELDS: Record<TAnalyticsTabsBase, readonly IInsightField[]> = { +} as const;A tiny tweak, but it tightens the type-safety contract for downstream code.
web/core/components/analytics/total-insights.tsx (2)
83-87: Avoid recomputing the modulo expression on every render
ANALYTICS_INSIGHTS_FIELDS[analyticsType]?.lengthis evaluated twice during each render just to decide the grid size.
Cache the length once for readability and a (micro) performance win.- !peekView - ? ANALYTICS_INSIGHTS_FIELDS[analyticsType]?.length % 5 === 0 - ? "gap-10 lg:grid-cols-5" - : "gap-8 lg:grid-cols-4" + !peekView + ? insightsLen % 5 === 0 + ? "gap-10 lg:grid-cols-5" + : "gap-8 lg:grid-cols-4"Place
const insightsLen = ANALYTICS_INSIGHTS_FIELDS[analyticsType]?.length ?? 0;just above the return.
89-97: Minor: extractinsightsinto auseMemofor stabilityMapping over
ANALYTICS_INSIGHTS_FIELDS[...]directly means a new array reference each render, which can cause unnecessary child reconciling. A quick memoisation keeps the reference stable until its dependencies change.- {ANALYTICS_INSIGHTS_FIELDS[analyticsType]?.map((item) => ( + {insights.map((item) => (const insights = useMemo( () => ANALYTICS_INSIGHTS_FIELDS[analyticsType] ?? [], [analyticsType] );Not critical, but helps when the list grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/constants/src/analytics/common.ts(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/analytics/page.tsx(2 hunks)web/ce/components/analytics/tabs.ts(0 hunks)web/ce/components/analytics/tabs.tsx(1 hunks)web/core/components/analytics/total-insights.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- web/ce/components/analytics/tabs.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/constants/src/analytics/common.ts (1)
packages/types/src/analytics.d.ts (1)
TAnalyticsTabsBase(5-5)
web/core/components/analytics/total-insights.tsx (1)
packages/constants/src/analytics/common.ts (1)
ANALYTICS_INSIGHTS_FIELDS(14-95)
web/app/(all)/[workspaceSlug]/(projects)/analytics/page.tsx (2)
web/ce/components/analytics/tabs.tsx (1)
getAnalyticsTabs(4-10)packages/i18n/src/store/index.ts (1)
t(211-232)
web/ce/components/analytics/tabs.tsx (3)
packages/i18n/src/store/index.ts (1)
t(211-232)web/core/components/analytics/overview/root.tsx (1)
Overview(19-19)web/core/components/analytics/work-items/root.tsx (1)
WorkItems(19-19)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
web/core/components/analytics/total-insights.tsx (1)
5-5: Import rename looks goodThe switch to
ANALYTICS_INSIGHTS_FIELDSaligns with the new constant name – no further action needed.web/app/(all)/[workspaceSlug]/(projects)/analytics/page.tsx (1)
43-44: Memo dependency list should includetor drop the outer memo
getAnalyticsTabs(t)already depends ont, butANALYTICS_TABSis memoised with[t], while the seconduseMemodepends onANALYTICS_TABSandrouter.
SinceANALYTICS_TABSis recreated whentchanges, the second memo will also update – the current setup is fine. Just ensure ESLint’sreact-hooks/exhaustive-depsrule stays green; otherwise, addtexplicitly to the second dependency array.
…slation for "no_of" in Russian
There was a problem hiding this comment.
Added this translation which got missed in last PR
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/i18n/src/locales/ru/translations.json(1 hunks)web/core/components/analytics/analytics-wrapper.tsx(1 hunks)web/core/components/analytics/overview/root.tsx(1 hunks)web/core/components/analytics/work-items/root.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/core/components/analytics/work-items/root.tsx
- packages/i18n/src/locales/ru/translations.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/core/components/analytics/analytics-wrapper.tsx (2)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)packages/i18n/src/store/index.ts (1)
t(211-232)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
web/core/components/analytics/overview/root.tsx (1)
8-8:❓ Verification inconclusive
i18n key existence check
Good move switching to a translation key.
Please double-check that"common.overview"is present in every locale file, otherwise the fallback behaviour will simply render the key string.
🏁 Script executed:
#!/bin/bash # Quickly grep for the key in all locale JSON files fd -e json -d 3 | xargs rg -n '"common.overview"'Length of output: 51
🏁 Script executed:
#!/bin/bash # Recursively search all JSON and YAML locale files for the translation key "common.overview" rg -n '"common.overview"' -g '*.json' -g '*.yml' -g '*.yaml'Length of output: 62
Please verify the
common.overviewkey in all locale files
I ran a repo-wide grep for"common.overview"in JSON/YAML files and found no matches. Make sure this key exists in every locale (for example undersrc/locales/{en,fr,…}/common.json), otherwise users will see the raw key string at runtime.To double-check, you can run:
rg -n '"common\.overview"' -g '*.json' -g '*.yml' -g '*.yaml'web/core/components/analytics/analytics-wrapper.tsx (2)
3-4: Nice addition ofuseTranslationhookImporting
useTranslationand centralising translation in the wrapper keeps pages clean.
No issues spotted here.
12-18:✅ Verification successful
Verify all call-sites migrated
Now that the prop name changed, ensure no old
title="…"usages remain.
🏁 Script executed:
#!/bin/bash # Search for outdated prop usage across repo rg -n '<AnalyticsWrapper[^>]+title='Length of output: 38
Continuing verification to catch multi-line usages:
🏁 Script executed:
#!/bin/bash # List all AnalyticsWrapper call-sites with surrounding context to check for any remaining `title` props rg -n '<AnalyticsWrapper' -C5Length of output: 2052
✅ All AnalyticsWrapper Call-Sites Updated
No instances of the oldtitle="…"prop remain—every usage now correctly usesi18nTitle.
…nslation key and improve layout
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/propel/src/charts/bar-chart/root.tsx(1 hunks)web/core/components/analytics/work-items/created-vs-resolved.tsx(1 hunks)web/core/components/analytics/work-items/priority-chart.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/components/analytics/work-items/created-vs-resolved.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/core/components/analytics/work-items/priority-chart.tsx (1)
packages/i18n/src/store/index.ts (1)
t(211-232)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
packages/propel/src/charts/bar-chart/root.tsx (1)
127-129: Good addition, but update typing sooffset/dxaren’t silentlyany
TBarChartProps['yAxis']currently doesn’t exposeoffsetanddx, so downstream callers need a type-cast or get an implicitany. Add them as optional numbers in the shared type (and bump the constant defaults here accordingly).Would you like me to open a follow-up PR with the type additions?
…ction for improved type safety
…out in DataTable component
pushya22
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
Type of Change
Summary by CodeRabbit