[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 (
|
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:Details
❓ 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:Details
✅ 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
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
* refactor: move web utils to packages * fix: build and lint errors * chore: update drag handle plugin * chore: update table cell type to fix build errors * fix: build errors * chore: sync few changes * feat: add pi base url to constants * chore: update all util imports in ee folder * chore: refactor web utils imports * chore: update imports * fix: build errors * fix: build errors * fix: update utils import * chore: minor fixes related to duplicate assets imports * chore: update duplicate assets service * [WEB-4316] chore: new endpoints to download an asset (#7207) * chore: new endpoints to download an asset * chore: add exception handling * [WEB-4323] refactor: Analytics refactor (#7213) * chore: updated label for epics * chore: improved export logic * refactor: move csvConfig to export.ts and clean up export logic * refactor: remove unused CSV export logic from WorkItemsInsightTable component * refactor: streamline data handling in InsightTable component for improved rendering * feat: add translation for "No. of {entity}" and update priority chart y-axis label to use new translation * refactor: cleaned up some component and added utilitites * feat: add "at_risk" translation to multiple languages in translations.json files * refactor: update TrendPiece component to use new status variants for analytics * fix: adjust TrendPiece component logic for on-track and off-track status * refactor: use nullish coalescing operator for yAxis.dx in line and scatter charts * feat: add "at_risk" translation to various languages in translations.json files * feat: add "no_of" translation to various languages in translations.json files * feat: update "at_risk" translation in Ukrainian, Vietnamese, and Chinese locales in translations.json files * refactor: rename insightsFields to ANALYTICS_INSIGHTS_FIELDS and update analytics tab import to use getAnalyticsTabs function * feat: update AnalyticsWrapper to use i18n for titles and add new translation for "no_of" in Russian * fix: update yAxis labels and offsets in various charts to use new translation key and improve layout * feat: define AnalyticsTab interface and refactor getAnalyticsTabs function for improved type safety * fix: update AnalyticsTab interface to use TAnalyticsTabsBase for improved type safety * fix: add whitespace-nowrap class to TableHead for improved header layout in DataTable component * [WEB-4311] fix: membership data handling and state reversal on error (#7205) * [WEB-4231] Pie chart tooltip #7192 * fix: build errors * fix: utils imports * chore: fix build errors * yarn lock file update --------- Co-authored-by: Aaryan Khandelwal <65252264+aaryan610@users.noreply.github.com> Co-authored-by: JayashTripathy <76092296+JayashTripathy@users.noreply.github.com> Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
* chore: updated label for epics
* chore: improved export logic
* refactor: move csvConfig to export.ts and clean up export logic
* refactor: remove unused CSV export logic from WorkItemsInsightTable component
* refactor: streamline data handling in InsightTable component for improved rendering
* feat: add translation for "No. of {entity}" and update priority chart y-axis label to use new translation
* refactor: cleaned up some component and added utilitites
* feat: add "at_risk" translation to multiple languages in translations.json files
* refactor: update TrendPiece component to use new status variants for analytics
* fix: adjust TrendPiece component logic for on-track and off-track status
* refactor: use nullish coalescing operator for yAxis.dx in line and scatter charts
* feat: add "at_risk" translation to various languages in translations.json files
* feat: add "no_of" translation to various languages in translations.json files
* feat: update "at_risk" translation in Ukrainian, Vietnamese, and Chinese locales in translations.json files
* refactor: rename insightsFields to ANALYTICS_INSIGHTS_FIELDS and update analytics tab import to use getAnalyticsTabs function
* feat: update AnalyticsWrapper to use i18n for titles and add new translation for "no_of" in Russian
* fix: update yAxis labels and offsets in various charts to use new translation key and improve layout
* feat: define AnalyticsTab interface and refactor getAnalyticsTabs function for improved type safety
* fix: update AnalyticsTab interface to use TAnalyticsTabsBase for improved type safety
* fix: add whitespace-nowrap class to TableHead for improved header layout in DataTable component
Type of Change
Summary by CodeRabbit