[Customer Portal][FE][Web] Add Dark Mode Support for Dashboard Charts and Improve Onboarding Visibility - #543
Conversation
Introduce shouldHideOnboardingData utility (and NOT_APPLICABLE_ONBOARDING_STATUS) to detect when onboarding status is "Not-Applicable" by normalizing the input; add unit tests covering true/false cases. Also replace Maximize2/Minimize2 with ChevronUp/ChevronDown in CaseDetailsTabs to update the focus-mode icons. Files changed: permission.ts, permission.test.ts, CaseDetailsTabs.tsx.
Use shouldHideOnboardingData to hide onboarding status/UI in ProjectInformationCard and ServiceHoursStatCards; add optional hideOnboardingStatus prop to project details types. Prevent mutating SECURITY_PAGE_TABS by copying before filtering in SecurityPage. Remove CS Manager field from CaseDetailsDetailsPanel and update tests to expect its absence and adjust the error message assertion.
Enable dark-mode styling for the Outstanding Incidents chart and make onboarding UI conditionally hidden. Changes: - dashboard: add DASHBOARD_CHART_DARK_MODE_SHADE constant and useDarkMode to switch slice/legend colors and center text color for dark mode; build a darkModeChartSource and use it for pie slices and legend rows. - project metadata: forward new hideOnboardingStatus prop from ProjectMetadata to ProjectMetadataSecondaryRow and implement conditional rendering (default false) to hide the onboarding status column when requested. - tests: add a test to verify onboarding status is hidden when hideOnboardingStatus is true. - service hours: import shouldHideOnboardingData and hide the onboarding hours section when permissions indicate it should be hidden. These updates improve visual consistency for dark themes and allow onboarding information to be suppressed based on props/permissions.
Enable dark-mode styling for ActiveCasesChart and CasesTrendChart. Import useDarkMode and compute isDarkMode, add dark-mode color maps (with fallbacks) to override slice and legend colors, and use displayChartData/displayLegendData when dark mode is active. Also adjust center text color in dark mode and normalize category names for matching in CasesTrendChart. Minor JSX formatting/Box sx adjustments applied for consistent layout.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR introduces dark mode color support for dashboard charts (ActiveCasesChart, CasesTrendChart, OutstandingIncidentsChart) by remapping chart colors based on a dark-mode shade constant. Additionally, a new permission utility function conditionally hides onboarding-related UI sections across project details components when the onboarding status is "Not-Applicable". Minor icon and logic updates are also included in case details and security pages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/customer-portal/webapp/src/features/project-details/components/time-tracking/ServiceHoursStatCards.tsx (1)
65-73:⚠️ Potential issue | 🟡 MinorGrid layout becomes lopsided when the onboarding card is hidden.
The outer
BoxusesgridTemplateColumns: { xs: "1fr", md: "1fr 1fr" }. WhenhideOnboardingCardis true, only the Query Hours card renders, so onmd+it occupies the left half with an empty right column — the card ends up visibly half-width instead of spanning the row. Consider collapsing to a single column when the onboarding card is hidden:♻️ Proposed tweak
<Box sx={{ display: "grid", - gridTemplateColumns: { xs: "1fr", md: "1fr 1fr" }, + gridTemplateColumns: { + xs: "1fr", + md: hideOnboardingCard ? "1fr" : "1fr 1fr", + }, gap: 2, mb: 3, }} >Also applies to: 122-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/features/project-details/components/time-tracking/ServiceHoursStatCards.tsx` around lines 65 - 73, The gridTemplateColumns on the outer Box causes a half-width empty column when hideOnboardingCard is true; update the Box's sx to set gridTemplateColumns conditionally based on hideOnboardingCard (e.g., use a single-column value for md when hideOnboardingCard is true and the two-column value when false) so the Query Hours card spans the full width; apply the same conditional change to the other Box usage in the component (the block around lines 122-181) and reference the hideOnboardingCard prop and the ServiceHoursStatCards component when making the change.
🧹 Nitpick comments (3)
apps/customer-portal/webapp/src/features/security/pages/SecurityPage.tsx (1)
60-74: Optional: the shallow copy is unnecessary.
Array.prototype.filteralready returns a new array, and in theelsebranchallTabsis only read, never mutated. You can returnSECURITY_PAGE_TABSdirectly without defeating theas constreadonly intent of the constant.♻️ Proposed simplification
const tabs = useMemo( - () => { - const allTabs = [...SECURITY_PAGE_TABS]; - return areFeaturePermissionsReady - ? allTabs.filter((tab) => - tab.id === SecurityTabId.VULNERABILITIES - ? getProjectPermissions(projectDetails?.type?.label, { - projectFeatures, - }).hasSecurityReportAnalysis - : true, - ) - : allTabs; - }, + () => + areFeaturePermissionsReady + ? SECURITY_PAGE_TABS.filter((tab) => + tab.id === SecurityTabId.VULNERABILITIES + ? getProjectPermissions(projectDetails?.type?.label, { + projectFeatures, + }).hasSecurityReportAnalysis + : true, + ) + : SECURITY_PAGE_TABS, [areFeaturePermissionsReady, projectDetails?.type?.label, projectFeatures], );That said, the current version is safe — it correctly avoids mutating the shared
SECURITY_PAGE_TABSmodule-level constant, which matches the PR's stated intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/features/security/pages/SecurityPage.tsx` around lines 60 - 74, The local shallow copy of SECURITY_PAGE_TABS is unnecessary; in the useMemo callback remove the spread ([...SECURITY_PAGE_TABS]) and use SECURITY_PAGE_TABS directly, returning SECURITY_PAGE_TABS in the else branch instead of allTabs so you don't defeat the readonly/as const intent; keep the existing filter logic that checks areFeaturePermissionsReady and SecurityTabId.VULNERABILITIES with getProjectPermissions(projectDetails?.type?.label, { projectFeatures }).hasSecurityReportAnalysis.apps/customer-portal/webapp/src/features/support/components/case-details/header/CaseDetailsTabs.tsx (1)
20-20: Icon semantics: verify chevron direction aligns with user mental model.The icon change from
Maximize2/Minimize2toChevronUp/ChevronDownis implemented correctly and thearia-labelproperly describes the action. However, chevron semantics are directional and may be less immediately intuitive than maximize/minimize icons for toggling focus mode—unless the visual behavior clearly supports the metaphor (e.g., focus mode visually "lifts" the interface up, or collapses content downward when exiting).Consider confirming with UX that the chevron directions feel natural to users, or adding a brief user-facing tooltip if the directional cue might be ambiguous.
Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/features/support/components/case-details/header/CaseDetailsTabs.tsx` at line 20, The chevron icons (ChevronUp/ChevronDown) in CaseDetailsTabs.tsx may be ambiguous for toggling focus mode; confirm with UX that the up/down directions map to entering/exiting focus, and if ambiguous add a brief tooltip or title text to the toggle button and ensure the aria-label on that button matches the tooltip; locate the toggle control using the imported ChevronUp/ChevronDown symbols and the focus toggle handler (e.g., the method or prop that flips focus mode) and update the button to include a tooltip/title and matching aria-label text so users get both directional and textual cues.apps/customer-portal/webapp/src/utils/__tests__/permission.test.ts (1)
189-197: Consider expanding edge-case coverage forshouldHideOnboardingData.The two assertions exercise only the exact happy/negative strings. The implementation explicitly normalizes via
?? "",trim(), andtoLowerCase(), but none of those code paths are covered. A few quick cases would lock in behavior and prevent regressions:♻️ Suggested additions
describe("shouldHideOnboardingData", () => { it("returns true when onboarding status is Not-Applicable", () => { expect(shouldHideOnboardingData("Not-Applicable")).toBe(true); }); it("returns false for active onboarding values", () => { expect(shouldHideOnboardingData("In Progress")).toBe(false); }); + + it("normalizes casing and surrounding whitespace", () => { + expect(shouldHideOnboardingData(" not-applicable ")).toBe(true); + expect(shouldHideOnboardingData("NOT-APPLICABLE")).toBe(true); + }); + + it("returns false for null, undefined, and empty input", () => { + expect(shouldHideOnboardingData(null)).toBe(false); + expect(shouldHideOnboardingData(undefined)).toBe(false); + expect(shouldHideOnboardingData("")).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/webapp/src/utils/__tests__/permission.test.ts` around lines 189 - 197, Add unit tests for shouldHideOnboardingData that cover the normalization code paths: assert it returns true when passed null or undefined (to exercise the `?? ""` branch), when passed strings with extra whitespace (to exercise `trim()`), and when passed different-cased variants like "not-applicable" or "NOT-APPLICABLE" (to exercise `toLowerCase()`), plus an empty string case; update the describe block around shouldHideOnboardingData to include these edge-case it() assertions to lock behavior and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/customer-portal/webapp/src/features/dashboard/components/charts/CasesTrendChart.tsx`:
- Around line 71-114: The dark-mode color lookup currently lowercases category
names but does not normalize spacing/hyphens/underscores, so update
CasesTrendChart to use a single normalization helper (e.g., normalizeCategory)
and apply it both when constructing darkModeColorByCategory and when doing the
lookup in displayChartData: trim, lowercase, and replace sequences of spaces,
underscores, or hyphens with a single hyphen or single space (pick one
consistent separator), then use that normalized key for map.set entries (for
"onboarding", "follow-up", etc.) and for darkModeColorByCategory.get(entry.name)
so variants like " Follow-up ", "follow_up", or "follow - up" resolve to the
same color.
In
`@apps/customer-portal/webapp/src/features/project-details/components/project-overview/project-information/ProjectMetadataSecondaryRow.tsx`:
- Line 109: The empty Grid rendered when hideOnboardingStatus is true uses
size={{ xs: 12, md: 4 }}, which creates a full-width blank row on mobile; update
the placeholder in ProjectMetadataSecondaryRow so it does not occupy space on
small screens by only reserving the column for md+ breakpoints (e.g., change the
Grid sizing from xs:12 to xs:0 or use a display prop to hide on xs and show on
md) while keeping md:4, and keep the conditional tied to hideOnboardingStatus so
the placeholder only appears on desktop.
- Around line 126-128: In ProjectMetadataSecondaryRow.tsx the ternary combines
isLoading and isError so errors render a Skeleton; split the conditions to
mirror the Go Live Date pattern: check isLoading first (render Skeleton), then
isError (render ErrorIndicator), then onboardingStatus (render the status UI).
Update the JSX around the isLoading/isError/onboardingStatus checks in the
ProjectMetadataSecondaryRow component so isError displays ErrorIndicator instead
of the Skeleton.
In
`@apps/customer-portal/webapp/src/features/support/components/case-details/details-tab/__tests__/CaseDetailsDetailsPanel.test.tsx`:
- Line 137: The test currently asserts expect(screen.queryByText("CS
Manager")).not.toBeInTheDocument() but uses the default fixture where csManager
is null, so seed the test's fixture with a representative non-null csManager
value (e.g., provide a mock user or name) before rendering
CaseDetailsDetailsPanel in CaseDetailsDetailsPanel.test.tsx so the negative
assertion is meaningful; update the setup that constructs the props/fixture (the
variable used to render the component in this test) to include a non-null
csManager and then keep the existing assertion to verify the label/value are
absent.
In `@apps/customer-portal/webapp/src/utils/permission.ts`:
- Line 47: The check against NOT_APPLICABLE_ONBOARDING_STATUS should collapse
separators so variants like "Not Applicable" or "N/A" normalize to
"not-applicable"; update the normalization used where you compare labels (the
code that currently does label.trim().toLowerCase()) to use
label.trim().toLowerCase().replace(/[\s-_]+/g, "-") instead, and change any
direct comparisons to use this normalized value (including the logic referencing
NOT_APPLICABLE_ONBOARDING_STATUS and the similar checks around the block
currently at lines 221-226).
---
Outside diff comments:
In
`@apps/customer-portal/webapp/src/features/project-details/components/time-tracking/ServiceHoursStatCards.tsx`:
- Around line 65-73: The gridTemplateColumns on the outer Box causes a
half-width empty column when hideOnboardingCard is true; update the Box's sx to
set gridTemplateColumns conditionally based on hideOnboardingCard (e.g., use a
single-column value for md when hideOnboardingCard is true and the two-column
value when false) so the Query Hours card spans the full width; apply the same
conditional change to the other Box usage in the component (the block around
lines 122-181) and reference the hideOnboardingCard prop and the
ServiceHoursStatCards component when making the change.
---
Nitpick comments:
In `@apps/customer-portal/webapp/src/features/security/pages/SecurityPage.tsx`:
- Around line 60-74: The local shallow copy of SECURITY_PAGE_TABS is
unnecessary; in the useMemo callback remove the spread ([...SECURITY_PAGE_TABS])
and use SECURITY_PAGE_TABS directly, returning SECURITY_PAGE_TABS in the else
branch instead of allTabs so you don't defeat the readonly/as const intent; keep
the existing filter logic that checks areFeaturePermissionsReady and
SecurityTabId.VULNERABILITIES with
getProjectPermissions(projectDetails?.type?.label, { projectFeatures
}).hasSecurityReportAnalysis.
In
`@apps/customer-portal/webapp/src/features/support/components/case-details/header/CaseDetailsTabs.tsx`:
- Line 20: The chevron icons (ChevronUp/ChevronDown) in CaseDetailsTabs.tsx may
be ambiguous for toggling focus mode; confirm with UX that the up/down
directions map to entering/exiting focus, and if ambiguous add a brief tooltip
or title text to the toggle button and ensure the aria-label on that button
matches the tooltip; locate the toggle control using the imported
ChevronUp/ChevronDown symbols and the focus toggle handler (e.g., the method or
prop that flips focus mode) and update the button to include a tooltip/title and
matching aria-label text so users get both directional and textual cues.
In `@apps/customer-portal/webapp/src/utils/__tests__/permission.test.ts`:
- Around line 189-197: Add unit tests for shouldHideOnboardingData that cover
the normalization code paths: assert it returns true when passed null or
undefined (to exercise the `?? ""` branch), when passed strings with extra
whitespace (to exercise `trim()`), and when passed different-cased variants like
"not-applicable" or "NOT-APPLICABLE" (to exercise `toLowerCase()`), plus an
empty string case; update the describe block around shouldHideOnboardingData to
include these edge-case it() assertions to lock behavior and prevent
regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afd476a1-76a8-40d4-a045-247cbcc36925
📒 Files selected for processing (17)
apps/customer-portal/webapp/src/features/dashboard/components/charts/ActiveCasesChart.tsxapps/customer-portal/webapp/src/features/dashboard/components/charts/CasesTrendChart.tsxapps/customer-portal/webapp/src/features/dashboard/components/charts/OutstandingIncidentsChart.tsxapps/customer-portal/webapp/src/features/dashboard/constants/charts.tsapps/customer-portal/webapp/src/features/project-details/components/ProjectInformationCard.tsxapps/customer-portal/webapp/src/features/project-details/components/project-overview/project-information/ProjectMetadata.tsxapps/customer-portal/webapp/src/features/project-details/components/project-overview/project-information/ProjectMetadataSecondaryRow.tsxapps/customer-portal/webapp/src/features/project-details/components/project-overview/project-information/__tests__/ProjectMetadata.test.tsxapps/customer-portal/webapp/src/features/project-details/components/project-overview/service-hours-allocations/ServiceHoursAllocationsCard.tsxapps/customer-portal/webapp/src/features/project-details/components/time-tracking/ServiceHoursStatCards.tsxapps/customer-portal/webapp/src/features/project-details/types/projectDetailsComponents.tsapps/customer-portal/webapp/src/features/security/pages/SecurityPage.tsxapps/customer-portal/webapp/src/features/support/components/case-details/details-tab/CaseDetailsDetailsPanel.tsxapps/customer-portal/webapp/src/features/support/components/case-details/details-tab/__tests__/CaseDetailsDetailsPanel.test.tsxapps/customer-portal/webapp/src/features/support/components/case-details/header/CaseDetailsTabs.tsxapps/customer-portal/webapp/src/utils/__tests__/permission.test.tsapps/customer-portal/webapp/src/utils/permission.ts
💤 Files with no reviewable changes (1)
- apps/customer-portal/webapp/src/features/support/components/case-details/details-tab/CaseDetailsDetailsPanel.tsx
Introduce normalization helpers and broaden onboarding/category matching, adjust layouts and UI behavior, and update tests. - Type: make TabBar.tabs readonly. - Dashboard: add normalizeCategory and use it for category color mapping so names with spaces/underscores/case variants match consistently. - Permissions: add normalizeLabel and NOT_APPLICABLE_ONBOARDING_ALIASES; update shouldHideOnboardingData to use normalized aliases for robust detection of Not Applicable variants. - Project details: hideOnboardingStatus grid now reserves responsive space (hidden on xs) and show ErrorIndicator when isError; adjust loading/error rendering order. - Time tracking: ServiceHoursStatCards grid columns depend on hideOnboardingCard to avoid empty column. - Security page: simplify tabs useMemo to directly filter SECURITY_PAGE_TABS when permissions ready. - Support UI: add title attribute to focus mode button for accessibility; update CaseDetailsDetailsPanel test to pass csManager data. - Tests: extend permission tests to cover null/empty and varied Not Applicable formats. These changes improve robustness against variant input labels, fix responsive layout gaps, and tighten UX/accessibility and test coverage.
Change NOT_APPLICABLE_ONBOARDING_STATUS from 'not-applicable' to 'Not-Applicable' to match the expected value. Also reformat the shouldForceSeverityS4 function for readability (split assignment and multiline hasSeverityMatch call); no behavioral changes intended.
Update NOT_APPLICABLE_ONBOARDING_STATUS constant from "Not-Applicable" to "not-applicable" to match the expected backend/API value and avoid case-sensitive mismatches in permission/onboarding checks.
e65b8f6
into
wso2-open-operations:dev-app-customer-portal
Description
This pull request introduces dark mode support for dashboard charts and improves the handling of onboarding status display in project details. The main changes involve updating the color schemes of charts and legends to better fit dark mode, and conditionally hiding onboarding information based on user permissions.
Dashboard Chart Dark Mode Support:
DASHBOARD_CHART_DARK_MODE_SHADEand logic to select appropriate color shades for chart slices and legends when dark mode is enabled inActiveCasesChart,CasesTrendChart, andOutstandingIncidentsChart. This affects both the pie chart and legend color rendering for improved accessibility and appearance in dark mode. [1] [2] [3] [4]useDarkModehook in chart components to determine if dark mode is active and adjust colors and center text styling accordingly. [1] [2] [3]Project Details Onboarding Status Handling:
shouldHideOnboardingDatautility to determine if onboarding status should be hidden based on permissions, and passed this flag throughProjectInformationCard,ProjectMetadata, andProjectMetadataSecondaryRowcomponents. This ensures onboarding status is only shown to authorized users. [1] [2] [3] [4] [5] [6]