Skip to content

Refactor evidence overview and policy components#238

Merged
carhartlewis merged 1 commit into
mainfrom
lewis/ui-fixes
Apr 1, 2025
Merged

Refactor evidence overview and policy components#238
carhartlewis merged 1 commit into
mainfrom
lewis/ui-fixes

Conversation

@carhartlewis

@carhartlewis carhartlewis commented Apr 1, 2025

Copy link
Copy Markdown
Contributor
  • Implemented a new layout for the evidence overview, including charts for evidence status and assignee data.
  • Created new components for evidence status and assignee charts, enhancing data visualization.
  • Removed deprecated evidence overview components and hooks to streamline the codebase.
  • Updated the loading state for evidence overview to improve user experience.
  • Adjusted routing and localization for evidence-related pages to ensure consistency across the application.

Summary by CodeRabbit

  • New Features

    • Introduced new visual charts for evidence tasks and statuses, enhancing dashboard insights.
    • Rolled out an updated evidence overview page with dynamic navigation and improved loading indicators.
  • Style

    • Streamlined navigation with updated routes to the evidence section.
    • Enforced a default dark theme for a consistent user experience.
  • Refactor

    • Removed legacy visualization components and simplified user menu options.
    • Enhanced UI labels for clearer evidence-related information.

- Implemented a new layout for the evidence overview, including charts for evidence status and assignee data.
- Created new components for evidence status and assignee charts, enhancing data visualization.
- Removed deprecated evidence overview components and hooks to streamline the codebase.
- Updated the loading state for evidence overview to improve user experience.
- Adjusted routing and localization for evidence-related pages to ensure consistency across the application.
@vercel

vercel Bot commented Apr 1, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2025 1:31am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
comp-portal ⬜️ Skipped (Inspect) Apr 1, 2025 1:31am

@coderabbitai

coderabbitai Bot commented Apr 1, 2025

Copy link
Copy Markdown

Walkthrough

This pull request applies cosmetic formatting adjustments in the policy update action while reworking several aspects of the evidence dashboard. New evidence visualization components have been added, including bar and pie charts with associated layout and data retrieval functions. Outdated and redundant evidence UI components have been removed. Additionally, minor updates to the policies overview (adding a loading fallback), theme settings in providers, and navigation routes in the main and user menus have been implemented, along with updates to localized text for the evidence feature.

Changes

File(s) Change Summary
apps/app/src/actions/policies/update-policy-action.ts Formatting adjustments (indentation switched from spaces to tabs) without changing logic.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-assignee-chart.tsx
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-status-chart.tsx
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/data/getEvidenceDashboard.ts
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/layout.tsx
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/loading.tsx
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/page.tsx
Introduced new evidence dashboard components for visualizing data (bar chart, pie chart), structured layout, and data fetching.
Multiple removed files (using ellipsis for brevity):
.../evidence/overview/components/AnimatedBar.tsx
.../evidence/overview/components/AssigneeChart/AssigneeBarChart.tsx
.../evidence/overview/components/AssigneeChart/AssigneeChartRow.tsx
.../evidence/overview/components/DepartmentChart/DepartmentBarChart.tsx
.../evidence/overview/components/DepartmentChart/DepartmentChartRow.tsx
.../evidence/overview/components/EvidenceOverview.tsx
.../evidence/overview/components/EvidenceUIStates.tsx
.../evidence/overview/components/FrameworkChart/FrameworkBarChart.tsx
.../evidence/overview/components/FrameworkChart/FrameworkChartRow.tsx
.../evidence/overview/components/HorizontalBarChart.tsx
.../evidence/overview/data/getEvidenceDashboard.ts (old version)
.../evidence/overview/layout.tsx (old version)
.../evidence/overview/loading.tsx (old version)
.../evidence/page.tsx
Removed outdated evidence UI components and supporting files no longer used after the dashboard refactor.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/page.tsx Updated PoliciesOverview to include a Suspense fallback rendering the Loading component.
apps/app/src/app/[locale]/providers.tsx
apps/app/src/components/main-menu.tsx
apps/app/src/components/user-menu.tsx
Modified theme settings to force a dark theme; updated navigation routes (e.g., evidence path changed) and removed theme/locale switch UI elements.
apps/app/src/locales/features/evidence.ts Updated localized text for the evidence dashboard (layout renaming, added keys for evidence status, and refined descriptions).

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant EP as EvidenceOverviewPage
    participant GD as getEvidenceDashboard
    participant ESC as EvidenceStatusChart
    participant EAC as EvidenceAssigneeChart
    U->>EP: Request Evidence Overview
    EP->>GD: Fetch evidence data
    GD-->>EP: Return dashboard data
    EP->>ESC: Render status chart with data
    EP->>EAC: Render assignee chart with data
    EP-->>U: Display Evidence Overview
Loading
sequenceDiagram
    participant U as User
    participant PO as PoliciesOverviewPage
    participant L as LoadingFallback
    U->>PO: Navigate to Policies Overview
    PO->>L: Activate fallback state (loading)
    PO-->>U: Render policies content once loaded
Loading

Possibly related PRs

Poem

I'm a rabbit in code, so quick on my feet,
Hopping through changes with a rhythmic beat.
New charts and layouts, all shiny and bright,
As old components vanish into the night.
With dark themes enforced and menus anew,
I celebrate these changes—oh how they grew!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (7)
apps/app/src/components/user-menu.tsx (1)

77-85: Remove Deprecated Commented-Out Code for Theme and Locale Switching
It looks like the JSX block for the theme and language switches (the <ThemeSwitch /> and <LocaleSwitch /> components) has been commented out instead of being removed. Since these UI elements have been deprecated as per the new design requirements, it would improve maintainability and clarity to remove these commented sections entirely.

Suggested diff to remove the commented code completely:

-            {/* <div className="flex flex-row justify-between items-center p-2">
-              <p className="text-sm">{t("user_menu.theme")}</p>
-              <ThemeSwitch />
-            </div>{" "}
-            <DropdownMenuSeparator />{" "}
-            <div className="flex flex-row justify-between items-center p-2">
-              <p className="text-sm">{t("user_menu.language")}</p>
-              <LocaleSwitch />
-            </div>{" "} */}
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/data/getEvidenceDashboard.ts (2)

43-45: Consider replacing the null check with an empty array check.

db.organizationEvidence.findMany() returns an array even if no records exist. Hence, checking if (!evidence) is always false. You may want to check if (evidence.length === 0) instead.


172-188: Remove or incorporate the unused statusCounts object for the first assignee.

The statusCounts object is incremented but never returned or otherwise used. This creates dead code. Consider removing it or returning it if needed for future usage.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/page.tsx (2)

37-39: Good authentication check, but consider error handling.

While checking if there's an organizationId is correct, you might want to gracefully handle errors if the database call fails before this check. For instance, if auth() throws, the code won't reach this redirect. A try/catch around the entire operation could provide better error feedback.


34-94: Consider adding robust error handling for database operations.

Although you wrap the code in a transaction, you still might benefit from handling exceptions (like network or DB errors). A retry mechanism or a user-friendly error message could provide better fault tolerance in production.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-assignee-chart.tsx (1)

63-64: Clarify the sort and slice logic.

You take the top 4 assignees by descending order and then reverse them—giving an ascending order. This works, but it might be clearer to keep them in descending order or rename the variables to reduce confusion.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-status-chart.tsx (1)

36-37: Unused "needs_review" color entry.

Although you've declared a needs_review color, the component never renders data for “needs review.” Double-check whether you intend to display “needs review” in this pie chart or remove the color entry if unused.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5e178b and cc3d32a.

📒 Files selected for processing (28)
  • apps/app/src/actions/policies/update-policy-action.ts (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-assignee-chart.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-status-chart.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/data/getEvidenceDashboard.ts (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/loading.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/page.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/list/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AnimatedBar.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AssigneeChart/AssigneeBarChart.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AssigneeChart/AssigneeChartRow.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/DepartmentChart/DepartmentBarChart.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/DepartmentChart/DepartmentChartRow.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/EvidenceOverview.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/EvidenceUIStates.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/FrameworkChart/FrameworkBarChart.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/FrameworkChart/FrameworkChartRow.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/HorizontalBarChart.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/data/getEvidenceDashboard.ts (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/layout.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/loading.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/page.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/page.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/page.tsx (1 hunks)
  • apps/app/src/app/[locale]/providers.tsx (1 hunks)
  • apps/app/src/components/main-menu.tsx (1 hunks)
  • apps/app/src/components/user-menu.tsx (1 hunks)
  • apps/app/src/locales/features/evidence.ts (1 hunks)
💤 Files with no reviewable changes (15)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/loading.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/page.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AnimatedBar.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/FrameworkChart/FrameworkBarChart.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AssigneeChart/AssigneeChartRow.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/data/getEvidenceDashboard.ts
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/layout.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/AssigneeChart/AssigneeBarChart.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/DepartmentChart/DepartmentBarChart.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/FrameworkChart/FrameworkChartRow.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/page.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/EvidenceOverview.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/DepartmentChart/DepartmentChartRow.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/EvidenceUIStates.tsx
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/overview/components/HorizontalBarChart.tsx
🧰 Additional context used
🧬 Code Definitions (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/layout.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/list/layout.tsx (1)
  • Layout (5-33)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/page.tsx (4)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/loading.tsx (1)
  • Loading (5-31)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-status-chart.tsx (1)
  • EvidenceStatusChart (39-177)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-assignee-chart.tsx (1)
  • EvidenceAssigneeChart (37-153)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/page.tsx (1)
  • generateMetadata (155-167)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/page.tsx (3)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/loading.tsx (1)
  • Loading (5-31)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/components/policy-status-chart.tsx (1)
  • PolicyStatusChart (56-199)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/components/policy-assignee-chart.tsx (1)
  • PolicyAssigneeChart (37-153)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/data/getEvidenceDashboard.ts (1)
apps/app/src/locales/features/evidence.ts (1)
  • evidence (1-35)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-assignee-chart.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/hooks/useAssigneeData.ts (1)
  • AssigneeData (8-14)
🔇 Additional comments (17)
apps/app/src/app/[locale]/providers.tsx (1)

24-25:

Details

❓ Verification inconclusive

Consider the accessibility implications of forcing a dark theme

The changes to defaultTheme="dark" and adding forcedTheme="dark" effectively remove user choice in theme selection by enforcing dark mode across the application. While this creates visual consistency, it may create accessibility issues for users who require different contrast modes or find dark themes harder to read.

According to the PR context, these changes align with the removal of theme selection UI elements from the UserMenu component, indicating an intentional decision to standardize on dark mode.

Have these changes been tested with accessibility tools and users with different visual needs? Consider allowing theme customization for accessibility purposes, even if dark is the default presentation.


🌐 Web query:

What are the accessibility considerations when enforcing a dark theme in web applications?

💡 Result:

Enforcing a dark theme in web applications requires careful consideration of accessibility to avoid unintended barriers for users. Here's a breakdown of key considerations and best practices:


1. Contrast and Readability Challenges

  • Minimum contrast ratios: Text must meet WCAG 2.1 AA standards (4.5:1 for normal text, 3:1 for large text). Pure black (#000000) and white (#FFFFFF) can create excessive glare, so dark grays (e.g., #121212) and off-whites (e.g., #E0E0E0) are preferable for reducing eye strain[3][9][16].
  • Font weight and style: Thin fonts may vanish on dark backgrounds, while bold fonts can appear overly heavy. Adjust font weight and color opacity to maintain clarity[4][16].
  • Saturated colors: Highly vibrant colors on dark themes often fail contrast checks. Use tools like Contrast Checker to validate compliance[3][9][10].

2. User Choice and Preferences

  • Avoid forced dark mode: Approximately 50% of users with astigmatism experience halation (blurred text) in dark mode due to iris dilation. Enforcing it universally can make content unreadable for them[6][7][16].
  • Provide a toggle: Allow users to switch between light/dark themes and respect system-level preferences via prefers-color-scheme CSS media queries[1][3][7].
  • Persist settings: Save user preferences in localStorage to prevent "flash of unstyled content" (FOUC) and ensure consistency[3][13].

3. Accessibility Compliance Risks

  • WCAG and ADA requirements: Dark themes must still comply with guidelines for color contrast, focus indicators, and non-text elements (e.g., buttons, icons). For example:
    • Visited links in default purple often lack contrast on dark backgrounds[7][12].
    • Focus outlines should invert colors (e.g., white borders on dark backgrounds)[7].
  • Color dependency: Never use color alone to convey information (e.g., red text for errors). Pair colors with text labels or icons[4][12].

4. Design and Technical Best Practices

  • Use CSS variables: Simplify theme switching with custom properties for colors, fonts, and transitions[3][13].
    :root {
      --bg-color: #ffffff;
      --text-color: #121212;
    }
    [data-theme="dark"] {
      --bg-color: #121212;
      --text-color: #E0E0E0;
    }
  • Adapt images/media: Apply filter: invert(1); to images in dark mode or provide alternative assets[13].
  • Test with assistive tools: Validate themes using screen readers (e.g., NVDA, VoiceOver) and contrast checkers like WebAIM[3][9].

5. Edge Cases and Platform Consistency

  • Browser/OS overrides: Some platforms (e.g., Samsung Internet) force dark themes, potentially clashing with app-specific designs. Guide users to disable these via browser settings[8][15].
  • Institutional branding: Custom brand colors may not adapt well to dark themes, requiring manual adjustments to meet contrast ratios[11].

Summary

While dark themes can reduce eye strain for some users and improve energy efficiency on OLED screens, accessibility hinges on user choice, contrast compliance, and inclusive testing. Prioritize flexibility by offering theme toggles, validate designs against WCAG standards, and avoid assumptions about universal preferences. As Stephanie Walter notes, “By designing for smaller groups, we raise the basement on expectations for all users”[7][16].

Citations:


Accessibility Check on Forced Dark Mode Implementation

In apps/app/src/app/[locale]/providers.tsx (lines 24–25), the implementation of

defaultTheme="dark"
forcedTheme="dark"

removes user choice by enforcing dark mode across the application. While standardizing the theme can simplify the UI—especially given the removal of theme selection from the UserMenu—this decision may impact accessibility.

Key considerations include:

  • Contrast Compliance: Ensure that color contrasts meet WCAG 2.1 AA guidelines. Dark backgrounds need appropriate text colors (beyond pure black/white) to avoid eye strain.
  • User Visual Needs: Forced dark mode might adversely affect users with astigmatism or those who rely on alternative contrast settings. Best practices suggest providing a toggle to respect user preference.
  • Accessibility Testing: Confirm that these changes have been tested with assistive tools (e.g., NVDA, VoiceOver) and checked for usability across diverse visual needs.

Given the intentional shift in design, please verify that thorough accessibility testing has been conducted and consider if fallback options or toggles might be necessary for compliance and broader user support.

apps/app/src/components/main-menu.tsx (1)

124-124: The simplified path for the evidence menu item looks good.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/loading.tsx (1)

1-31: The loading component implementation looks good.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/list/layout.tsx (1)

20-21: Path update aligns with centralized routing changes

The change from /${organizationId}/evidence/overview to /${organizationId}/evidence streamlines the routing structure, matching similar changes made in the MainMenu component. This ensures consistent navigation paths throughout the application.

apps/app/src/actions/policies/update-policy-action.ts (3)

10-15: Code formatting improvement

The indentation has been changed from spaces to tabs for better consistency. This formatting change maintains the same functionality while improving code readability.


20-54: Consistent tab-based indentation

Formatting changes from spaces to tabs have been applied throughout the function, maintaining consistent styling without altering the function's behavior.


57-117: Standardized indentation pattern

The indentation style has been updated to use tabs consistently throughout the updatePolicyAction export, which helps maintain a uniform code style across the codebase.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/layout.tsx (1)

1-33: Well-structured layout component with consistent navigation

This new layout component correctly implements the standard structure seen in other parts of the application. It properly retrieves user session data, handles internationalization, and renders a consistent navigation menu with appropriately structured routes.

The component mirrors the structure used in evidence/list/layout.tsx, ensuring consistency across related sections of the application.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/page.tsx (2)

9-9: Added loading fallback import

Good addition of the Loading component import which will be used to provide visual feedback during data loading.


22-22: Enhanced loading state with fallback

The addition of the Loading component as a fallback to the Suspense wrapper improves the user experience by providing visual feedback during data loading, consistent with the PR objective of addressing loading states.

The fallback component shows loading indicators in each card space, maintaining the same layout structure as the loaded content.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/page.tsx (1)

146-158: Metadata generation looks good.

Using getI18n() to fetch the localized title and setting static params for locale is straightforward, and advanced error handling isn't strictly necessary here. Great job.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/evidence/(overview)/components/evidence-status-chart.tsx (1)

79-80: Filtering out zero-value items.

By filtering out items with zero counts, the chart won’t display those statuses. This is perfectly fine if you want to avoid clutter, but confirm it’s the desired behavior (e.g., you might still want to show slices with “0” to emphasize no data).

apps/app/src/locales/features/evidence.ts (5)

7-7: Change from "Dashboard" to "Overview" improves consistency.

The update from "Dashboard" to "Overview" aligns with the property name overview in line 4 and the evidence_tasks object in line 38, creating better terminology consistency throughout the user interface.


10-10: More descriptive label for assignee chart.

Changing from "By Assignee" to "Evidence Tasks By Assignee" makes the purpose of this section clearer to users, supporting the new chart visualization mentioned in the PR description.


12-12: Added new localization key for evidence status visualization.

Adding the evidence_status key supports the new status chart component mentioned in the PR description, completing the necessary localization support for the new UI element.


16-16: New status values support enhanced filtering capabilities.

The addition of "Published" and "Is Not Relevant" status values expands the evidence classification options, which will improve data visualization and filtering in the new status chart component.

Also applies to: 21-21


38-38: Consistent naming with dashboard layout.

The "Overview" value in evidence_tasks aligns with the updated dashboard layout terminology in line 7, maintaining consistency throughout the application.

Comment on lines +117 to +129
// status = published if published is true
// status = draft if published is false and isNotRelevant is false
// status = isNotRelevant if published is false and isNotRelevant is true
// status = needsReview if published is true and needs review

if (evidence.published) {
assigneeData.published += 1;
} else if (evidence.isNotRelevant) {
assigneeData.isNotRelevant += 1;
} else {
// If not published and not irrelevant, it's a draft
assigneeData.draft += 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing logic for "needsReview" evidence.

The code comment implies a scenario where evidence is published but still requires review, yet there's no corresponding condition that increments assigneeData.needsReview. This can cause data inaccuracies on the chart indicating “needs review.”

} else if (evidence.isNotRelevant) {
  assigneeData.isNotRelevant += 1;
} else if (evidence.needsReview) {
  assigneeData.needsReview += 1;
} else {
  assigneeData.draft += 1;
}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +66 to +72
const chartData = sortedData.map((item) => ({
name: item.name,
published: item.published,
draft: item.draft,
archived: item.archived,
needs_review: item.needs_review,
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Validate property naming for "archived" vs "isNotRelevant".

The chart references archived but the database property is isNotRelevant. Ensure these concepts align correctly, or rename the property to maintain consistency across files.


🛠️ Refactor suggestion

Mismatch in naming "needs_review" vs "needsReview".

Your chart data references needs_review, but the backend logic increments needsReview. This discrepancy can cause zero-increment issues. Rename the properties to match one another for correct tallying.

- needs_review: item.needs_review,
+ needs_review: item.needsReview,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const chartData = sortedData.map((item) => ({
name: item.name,
published: item.published,
draft: item.draft,
archived: item.archived,
needs_review: item.needs_review,
}));
const chartData = sortedData.map((item) => ({
name: item.name,
published: item.published,
draft: item.draft,
archived: item.archived,
needs_review: item.needsReview,
}));

@carhartlewis carhartlewis merged commit 746b859 into main Apr 1, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Apr 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant