fix: update styles of charts on /insights#22557
Conversation
|
""" WalkthroughThis pull request refactors the layout and styling of multiple components in the insights module, particularly those used on the Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. All modifications relate directly to the objectives of updating chart styles, layouts, and related localization for the 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
packages/features/insights/components/BookingKPICards.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)packages/features/insights/components/BookingKPICards.tsx (3)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
|
✅ No security or compliance issues detected. Reviewed everything up to 9a95877. Security Overview
Detected Code Changes
Reply to this PR with |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/16/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/17/25)1 label was added to this PR based on Keith Williams's automation. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/web/public/static/locales/en/common.json (2)
2724-2726: Grammar / phrasing tweaks• “Most No Show” is awkward; consider “Most No-Shows” or “Highest No-Show Rate”.
• “Highest Rated” / “Lowest Rated” read fine, but double-check the surrounding UI copy (does it show members or events?) to avoid ambiguity.No functional issue, just polish.
3146-3146: Same truncation issue as above- "most_cancelled_bookings": "Most Cancelled", + "most_cancelled_bookings": "Most Cancelled Bookings",Keeps the wording parallel with the metric name and avoids confusion with “most cancelled events” vs anything else.
packages/features/insights/components/TotalBookingUsersTable.tsx (2)
20-44: Consider using semantic HTML for better accessibility.While the flexbox layout works visually, replacing table elements with divs may impact screen reader users and keyboard navigation. Consider using native table elements with Tailwind classes or adding appropriate ARIA attributes to maintain accessibility.
- <div className="overflow-hidden rounded-md"> - {filteredData.length > 0 ? ( - filteredData.map((item) => ( - <div - key={item.userId} - className="border-subtle flex items-center justify-between border-b px-4 py-3 last:border-b-0"> + <div className="overflow-hidden rounded-md"> + {filteredData.length > 0 ? ( + <table className="w-full"> + <tbody> + {filteredData.map((item) => ( + <tr + key={item.userId} + className="border-subtle border-b last:border-b-0"> + <td className="px-4 py-3"> + <div className="flex items-center"> + <Avatar + alt={item.user.name || ""} + size="sm" + imageSrc={getUserAvatarUrl({ avatarUrl: item.user.avatarUrl })} + title={item.user.name || ""} + className="mr-3" + /> + <div className="text-default text-sm font-medium">{item.user.name}</div> + </div> + </td> + <td className="px-4 py-3 text-right"> + <div className="text-default text-sm font-medium">{item.count}</div> + </td> + </tr> + ))} + </tbody> + </table>
40-42: Reconsider the fixed height for empty state.The fixed height
h-60(15rem) might not be appropriate in all contexts where this component is used. Consider making it more flexible or accepting it as a prop.- <div className="flex h-60 text-center"> + <div className="flex min-h-[10rem] py-8 text-center">packages/features/insights/components/FeedbackTable.tsx (1)
42-42: Add tooltip for truncated feedback text.The truncated feedback might hide important information. Consider adding a tooltip to show the full text on hover.
Add a Tooltip component import:
+ import { Tooltip } from "@calcom/ui/components/tooltip";Then wrap the truncated feedback:
- <div className="text-default max-w-xs truncate text-sm font-medium">{item.feedback}</div> + <Tooltip content={item.feedback}> + <div className="text-default max-w-xs truncate text-sm font-medium">{item.feedback}</div> + </Tooltip>packages/features/insights/components/TotalUserFeedbackTable.tsx (1)
42-43: Maintain consistency in empty state messages.The empty state message "No data found" differs from "No members found" in
TotalBookingUsersTable. Consider using consistent messaging across similar table components for better UX.- <p className="m-auto text-sm font-light">No data found</p> + <p className="m-auto text-sm font-light">No members found</p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/web/modules/insights/insights-view.tsx(1 hunks)apps/web/public/static/locales/en/common.json(3 hunks)packages/features/insights/components/AverageEventDurationChart.tsx(3 hunks)packages/features/insights/components/BookingKPICards.tsx(4 hunks)packages/features/insights/components/BookingStatusLineChart.tsx(3 hunks)packages/features/insights/components/FeedbackTable.tsx(2 hunks)packages/features/insights/components/HighestNoShowHostTable.tsx(2 hunks)packages/features/insights/components/HighestRatedMembersTable.tsx(2 hunks)packages/features/insights/components/KPICard.tsx(2 hunks)packages/features/insights/components/LeastBookedTeamMembersTable.tsx(2 hunks)packages/features/insights/components/LowestRatedMembersTable.tsx(2 hunks)packages/features/insights/components/MostBookedTeamMembersTable.tsx(2 hunks)packages/features/insights/components/MostCancelledBookingsTables.tsx(2 hunks)packages/features/insights/components/PopularEventsTable.tsx(2 hunks)packages/features/insights/components/RecentFeedbackTable.tsx(2 hunks)packages/features/insights/components/TotalBookingUsersTable.tsx(1 hunks)packages/features/insights/components/TotalUserFeedbackTable.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
packages/features/insights/components/AverageEventDurationChart.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/insights/components/HighestRatedMembersTable.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/insights/components/RecentFeedbackTable.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/insights/components/BookingStatusLineChart.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/LeastBookedTeamMembersTable.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/LowestRatedMembersTable.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/insights/components/MostBookedTeamMembersTable.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/MostCancelledBookingsTables.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/HighestNoShowHostTable.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/TotalBookingUsersTable.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
packages/features/insights/components/PopularEventsTable.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
apps/web/modules/insights/insights-view.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
apps/web/public/static/locales/en/common.json (1)
undefined
<retrieved_learning>
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
</retrieved_learning>
packages/features/insights/components/BookingKPICards.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
🧬 Code Graph Analysis (7)
packages/features/insights/components/AverageEventDurationChart.tsx (1)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)
packages/features/insights/components/HighestRatedMembersTable.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)packages/features/insights/components/TotalUserFeedbackTable.tsx (1)
TotalUserFeedbackTable(5-48)
packages/features/insights/components/BookingStatusLineChart.tsx (1)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)
packages/features/insights/components/LowestRatedMembersTable.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)packages/features/insights/components/TotalUserFeedbackTable.tsx (1)
TotalUserFeedbackTable(5-48)
packages/features/insights/components/MostBookedTeamMembersTable.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)packages/features/insights/components/TotalBookingUsersTable.tsx (1)
TotalBookingUsersTable(5-46)
packages/features/insights/components/MostCancelledBookingsTables.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)packages/features/insights/components/TotalBookingUsersTable.tsx (1)
TotalBookingUsersTable(5-46)
packages/features/insights/components/HighestNoShowHostTable.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(14-52)packages/features/insights/components/TotalUserFeedbackTable.tsx (1)
TotalUserFeedbackTable(5-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Check
🔇 Additional comments (25)
packages/features/insights/components/KPICard.tsx (1)
23-23: LGTM: Simplified wrapper aligns with component hierarchy.The change from
CardInsightsto plaindivis appropriate sinceKPICardis now used as a child component withinChartCardcontainers, avoiding nested card styling issues.Also applies to: 49-49
packages/features/insights/components/AverageEventDurationChart.tsx (2)
6-6: LGTM: Import added for new ChartCard component.
37-37: LGTM: Consistent refactoring to use ChartCard component.The change from
CardInsightswrapper with explicitTitlecomponent toChartCardwithtitleprop simplifies the JSX structure and follows the consistent refactoring pattern across the insights components.Also applies to: 53-53
packages/features/insights/components/RecentFeedbackTable.tsx (2)
5-5: LGTM: Import added for new ChartCard component.
34-34: LGTM: Consistent refactoring to use ChartCard component.The change from
CardInsightswrapper toChartCardwithtitleprop follows the consistent refactoring pattern across the insights components and simplifies the JSX structure.Also applies to: 36-36
packages/features/insights/components/HighestNoShowHostTable.tsx (2)
5-5: LGTM: Import added for new ChartCard component.
34-34: LGTM: Consistent refactoring to use ChartCard component.The change from
CardInsightswrapper toChartCardwithtitleprop follows the consistent refactoring pattern across the insights components and maintains the same functionality.Also applies to: 36-36
packages/features/insights/components/LeastBookedTeamMembersTable.tsx (2)
5-5: LGTM: Import added for new ChartCard component.
34-34: LGTM: Consistent refactoring to use ChartCard component.The change from
CardInsightswrapper toChartCardwithtitleprop completes the consistent refactoring pattern across the insights components and maintains the same functionality.Also applies to: 36-36
packages/features/insights/components/MostBookedTeamMembersTable.tsx (2)
5-5: Clean import update for UI standardization.The import correctly switches to the new
ChartCardcomponent as part of the insights UI refactor.
34-36: Well-structured component wrapper refactor.The transition from
CardInsights+ separateTitletoChartCardwithtitleprop simplifies the JSX structure while maintaining the same functionality. This change is consistent with the broader UI standardization effort across insights components.packages/features/insights/components/MostCancelledBookingsTables.tsx (2)
5-5: Import correctly updated for UI consistency.The
ChartCardimport aligns with the standardized card component approach across insights components.
32-34: Proper wrapper component refactor.The
ChartCardcorrectly wraps the conditional rendering logic while providing a consistent title display. The refactor maintains the original component behavior.packages/features/insights/components/LowestRatedMembersTable.tsx (2)
5-5: Import update maintains consistency.The
ChartCardimport follows the standardized pattern across all insights table components.
34-36: Consistent component wrapper transition.The
ChartCardwithtitleprop properly replaces the previousCardInsights+Titlepattern while maintaining the same rendering logic and functionality.packages/features/insights/components/HighestRatedMembersTable.tsx (2)
5-5: Import correctly aligned with UI refactor.The
ChartCardimport maintains consistency with the standardized card component approach.
34-36: Proper component wrapper update.The
ChartCardwithtitleprop correctly replaces the previous card structure while maintaining the same conditional rendering logic and functionality.packages/features/insights/components/BookingStatusLineChart.tsx (2)
6-6: Import update follows established pattern.The
ChartCardimport correctly aligns with the UI standardization across insights components.
40-49: Consistent chart wrapper refactor.The
ChartCardwithtitleprop properly replaces the previous card structure while maintaining theLineChartcomponent's configuration and rendering logic.apps/web/modules/insights/insights-view.tsx (2)
78-83: Grid layout improvement looks good.The change from 2 columns to 4 columns on large screens (
lg:grid-cols-4) will provide better space utilization for the team member tables. The responsive design maintains 1 column on mobile and 2 columns on medium screens, ensuring good usability across devices.
84-90: Smart layout organization for rating tables and feedback.The new grid structure effectively groups the rating tables with the feedback table, and the
lg:col-span-2on RecentFeedbackTable creates a balanced layout where the feedback table takes up half the width on large screens. This logical grouping improves the visual hierarchy.packages/features/insights/components/PopularEventsTable.tsx (2)
5-5: Component refactoring aligns with the broader UI standardization.The import change to
ChartCardis consistent with the broader effort to standardize card components across the insights module.
35-54: Effective transition from Tremor UI to custom styled components.The refactoring successfully replaces the Tremor UI table structure with a clean, custom div-based layout:
- Improved maintainability: Using
ChartCardwith atitleprop simplifies the component structure- Consistent styling: The flexbox layout with Tailwind classes provides consistent spacing and borders
- Preserved functionality: The conditional rendering for empty data and loading states remains intact
- Accessibility: The semantic structure is maintained despite moving away from table elements
The border utilities (
border-muted,border-b,last:border-b-0) create clean visual separation between items, and the responsive text sizing ensures good readability.apps/web/public/static/locales/en/common.json (2)
2170-2171: Value wording feels truncated – consider restoring the noun for clarityThe keys still reference “*_members”, yet the values have been shortened to “Most Booked” / “Least Booked”.
If the UI section still lists members, dropping the noun may confuse translators or future readers.- "most_booked_members": "Most Booked", - "least_booked_members": "Least Booked", + "most_booked_members": "Most Booked Members", + "least_booked_members": "Least Booked Members",(Same applies to any other locale files.)
[ suggest_nitpick ]
2176-2177: Very-generic keys risk future collisions
"events"and"performance"are top-level, extremely common words.
The file already contains >3 000 keys; a second team member could easily add another"events"elsewhere and silently override this one.Recommend namespacing, e.g.
- "events": "Events", - "performance": "Performance", + "insights_events_tab": "Events", + "insights_performance_tab": "Performance",Would you mind grepping the codebase for existing
"events"/"performance"keys to ensure you’re not shadowing an earlier definition?
8e0e2bb to
03e79c9
Compare
sean-brydon
left a comment
There was a problem hiding this comment.
This is looking great! Nice job!
E2E results are ready! |
What does this PR do?
This PR updates the overall style of charts on the /insights page.
Visual Demo (For contributors especially)
Before
Screenshot.2025-07-16.at.14.45.46.mp4
After
Screenshot.2025-07-16.at.17.48.53.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Visit /insights and it works the same as before.