Skip to content

Conversation

@gakshita
Copy link
Collaborator

@gakshita gakshita commented Jan 6, 2025

Summary

This PR implements manage widgets button on home page with integrations

Summary by CodeRabbit

Based on the comprehensive summary, here are the release notes:

  • New Features

    • Added a new workspace dashboard with customizable widgets
    • Introduced quick links management functionality
    • Implemented recent activity tracking across workspace entities
    • Added user greetings and personalized dashboard experience
  • Improvements

    • Enhanced type definitions for home and workspace-related data structures
    • Improved state management for workspace interactions
    • Added drag-and-drop functionality for widget management
  • Bug Fixes

    • Refined component rendering and state handling
    • Improved error handling in service and store implementations
  • Performance

    • Optimized data fetching and state updates using MobX and SWR
    • Implemented efficient widget reordering mechanisms

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2025

Walkthrough

This pull request introduces a comprehensive enhancement to the home dashboard functionality in a web application. It adds new type definitions, creates multiple React components for widgets and home views, and implements a robust state management system using MobX. The changes focus on improving the user experience by providing dynamic, interactive widgets for recent activities, quick links, and workspace management. The implementation includes drag-and-drop functionality, modal interactions, and responsive design elements.

Changes

File Change Summary
packages/types/src/home.d.ts Added multiple type definitions for home-related entities, including TRecentActivityFilterKeys, THomeWidgetKeys, TPageEntityData, TProjectEntityData, and more
packages/types/src/index.d.ts Added export for home module types
web/core/services/workspace.service.ts Added methods for managing workspace links, recents, and widgets
web/core/store/workspace/home.ts Introduced HomeStore for managing widget settings and interactions
web/core/components/home/* Added multiple components for dashboard widgets, including recent activities, quick links, and widget management

Sequence Diagram

sequenceDiagram
    participant User
    participant Dashboard
    participant HomeStore
    participant WorkspaceService
    
    User->>Dashboard: Open Workspace Home
    Dashboard->>HomeStore: Fetch Widgets
    HomeStore->>WorkspaceService: Request Widget Data
    WorkspaceService-->>HomeStore: Return Widget Configuration
    HomeStore-->>Dashboard: Update Widget State
    Dashboard->>User: Render Widgets
Loading

Possibly related PRs

Suggested Labels

🌟enhancement, 🌐frontend, 🎨UI / UX

Suggested Reviewers

  • SatishGandham
  • gurusainath

Poem

🐰 A Rabbit's Dashboard Delight 🌈

Widgets dance and links take flight,
MobX state, a programmer's might,
Drag and drop with pure delight,
Home dashboard, shining bright!
Code that makes the workspace sing! 🎉


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ 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. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (45)
web/core/components/home/root.tsx (4)

54-56: Centralize error handling for better maintainability

Here, errors are only logged with console.error. For a production environment, consider using a robust error tracking mechanism (e.g., Sentry or a custom logger) to record and handle errors more comprehensively.


60-66: Ensure focus management for the Tour modal

When the tour modal is displayed, consider managing keyboard focus (e.g., focusing on the first interactive element upon open) to enhance accessibility.


68-93: Remove unnecessary fragment

Static analysis hints that this fragment within the conditional block might be redundant. If there's only one child (the ternary), you can remove the fragment and return the JSX directly.

-        <>
           {joinedProjectIds.length > 0 || loader ? (
             ...
           ) : (
             ...
           )}
-        </>
🧰 Tools
🪛 Biome (1.9.4)

[error] 68-93: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)


70-84: Factor out nested fragments for cleaner structure

The nested fragments (<> ... </>) in the ternary operator can be replaced with a single wrapper or direct elements to reduce complexity and improve readability.

web/core/store/workspace/home.ts (2)

63-75: Improve error feedback strategy

Errors are logged but not surfaced to the UI. Consider returning user-friendly error messages or employing a global error-handling routine for consistency.


78-90: Ensure reactivity on widget toggles

It’s good that you're doing an async call and then updating state in a runInAction. As a minor enhancement, you could add local state feedback (e.g., a spinner or disabled UI) while the toggle is in-flight to improve user experience.

web/core/components/home/widgets/manage/widget-item.tsx (1)

78-111: Prevent unintended reorder states

The logic for “blockedStates” and different drop instructions effectively reduces invalid states. If new use cases arise (e.g., grouping widgets), consider extracting this into a utility for maintainability.

web/core/components/home/widgets/loaders/quick-links.tsx (1)

7-13: Parameterize loader items for flexibility

The loader is hardcoded to show four items. Consider allowing a dynamic number of items for future flexibility, for example by passing a count prop.

web/core/components/home/widgets/loaders/loader.tsx (1)

16-25: Consider handling unknown widget keys.
If an unsupported widgetKey is passed to WidgetLoader, the component returns undefined, leading to a blank render or error. Include a fallback or error boundary to prevent unexpected behavior.

Example fallback implementation:

  const loaders = {
    [EWidgetKeys.RECENT_ACTIVITY]: <RecentActivityWidgetLoader />,
    [EWidgetKeys.QUICK_LINKS]: <QuickLinksWidgetLoader />,
+   default: <div>Widget not found</div>
  };
  
  return loaders[widgetKey] ?? loaders.default;
web/core/components/home/widgets/links/action.tsx (1)

1-20: Minor UI label consistency suggestion.
The label "Add quick Link" uses mixed casing. Consider "Add Quick Link" for consistency with typical title casing or "Add quick link" for sentence casing. Also, ensure that an aria-label or descriptive text is used for accessibility.

- <div className="text-sm font-medium my-auto">Add quick Link</div>
+ <div className="text-sm font-medium my-auto">Add Quick Link</div>
web/core/components/home/widgets/manage/widget-item-drag-handle.tsx (1)

9-12: Unused prop sort_order
The sort_order prop is declared but not used. If it’s intended for future logic, consider leaving a comment about its upcoming usage; otherwise, please remove it to keep the component clean.

 type Props = {
-  sort_order: number | null;
   isDragging: boolean;
 };
web/app/[workspaceSlug]/(projects)/home/page.tsx (1)

15-15: Consider a fallback page title
Currently, if currentWorkspace?.name is missing, pageTitle is undefined. Adding a more user-friendly fallback (e.g., "Home") would improve clarity and possibly SEO.

-const pageTitle = currentWorkspace?.name ? `${currentWorkspace?.name} - Home` : undefined;
+const pageTitle = currentWorkspace?.name
+  ? `${currentWorkspace?.name} - Home`
+  : "Home";
web/core/components/home/widgets/manage/index.tsx (1)

20-35: Modal approach for managing widgets
Using a centralized modal to manage widget settings is a clean approach. Be mindful of potential concurrency or race conditions if multiple user sessions modify widgets simultaneously.

web/core/components/home/widgets/links/root.tsx (1)

16-24: Add error handling or fallback UI
When using SWR to fetch links, consider handling or surfacing errors. Currently, if fetchLinks fails, there is no fallback UI or error message for the user.

 useSWR(
   workspaceSlug ? `HOME_LINKS_${workspaceSlug}` : null,
   workspaceSlug ? () => fetchLinks(workspaceSlug.toString()) : null,
   {
     revalidateIfStale: false,
     revalidateOnFocus: false,
     revalidateOnReconnect: false,
+    onError: (error) => {
+      console.error("Failed to fetch links:", error);
+      // Optionally show error UI or toast
+    }
   }
 );
packages/types/src/home.d.ts (2)

40-46: Consider separating the entity types.

Right now, TActivityEntityData allows the entity to be a page, project, or issue. While this union type works, you may want to consider discriminated unions for better type safety. For example:

type TActivityEntityData =
  | { entity_name: "issue"; entity_data: TIssueEntityData; ... }
  | { entity_name: "project"; entity_data: TProjectEntityData; ... }
  ...

48-51: Provide a specific type for metadata.

metadata is typed as any. This can obscure the shape of the data and weaken type safety. Providing a dedicated interface or a more specific union for metadata enhances readability and avoids type confusion.

web/core/components/home/project-empty-state.tsx (1)

13-46: Provide a fallback for users without project creation permissions.

Currently, the component only shows a button if a user is an ADMIN. Consider rendering helpful guidance or a read-only message for team members lacking permissions, ensuring they understand why they cannot add a project.

web/core/components/home/widgets/recents/filters.tsx (1)

1-51: Ensure consistency in pluralization.

Appending "s" to filter.name at line 30 and 41 might produce awkward labels for certain filter keys (e.g., "pages" is correct, "projects" works, but "alls" is off if you eventually rename "all item" to "all"). Consider a small utility function handling singular/plural forms for filters more elegantly.

web/core/components/home/user-greetings.tsx (2)

1-11: Ensure consistent naming for types and props.

The IUserGreetingsView interface name and the prop names align well, but check if there's a more consistent naming convention across the codebase (e.g., some interfaces use an I prefix, and others don’t). This helps maintain a uniform style.


54-60: Enable finer access controls for Manage Widgets.

Depending on the user’s role, you may want to limit who can manage widgets. If relevant, implement a conditional check to hide or disable this button if the user lacks permission.

web/core/components/home/home-dashboard-widgets.tsx (1)

14-22: Encapsulate widget component definitions.

WIDGETS_LIST might benefit from its own configuration file or utility module to keep the code modular and maintainable, especially if more widgets are added in the future.

web/core/components/home/widgets/manage/widget-list.tsx (1)

30-44: Enhance user feedback for reorder operation.

The success and error toasts are useful. For a better user experience, consider a loading state or progress indicator during the reorder process. This would confirm that the system is processing the action before success or error toasts pop up.

web/core/components/home/widgets/manage/widget.helpers.ts (1)

4-9: Consider clarifying the naming of isGroup and isChild.
The properties isGroup and isChild can sometimes be confusing to those unfamiliar with your drag-and-drop model. Consider adding short doc comments to clarify their purpose, or rename them if you anticipate future complexity.

web/core/components/home/widgets/links/links.tsx (2)

30-51: Use a debounce or throttle for resize events.
Repeatedly firing updateColumnCount on rapid window resizes can degrade performance. Consider debouncing or throttling the resize handler to optimize.

 useEffect(() => {
-  window.addEventListener("resize", updateColumnCount);
+  const optimizedListener = debounce(updateColumnCount, 250);
+  window.addEventListener("resize", optimizedListener);
   ...
   return () => window.removeEventListener("resize", optimizedListener);
 }, []);

53-77: Prevent potential rendering edge cases with large link arrays.
The logic for showing a subset of links and toggling “Show more” is correct. However, if a user has exactly 2 * columnCount - 1 links, the button won't appear, and the user might not realize no more links exist. Consider clarifying or adjusting the display logic for that boundary condition.

web/core/components/home/widgets/recents/project.tsx (2)

8-12: Ref naming.
Naming the type property ref might cause confusion with native DOM ref. Although valid, consider a more domain-specific name like parentRef or containerRef to be more descriptive.


20-70: Avoid over-nesting and keep the UI minimal.
The rendering logic in <ListItem> is clear. The quick action for showing project members is well-structured. If the design grows more complex, consider extracting sub-components.

web/core/components/home/widgets/recents/index.tsx (3)

9-26: Confirm icon usage.
LayersIcon, FileText, Briefcase are effectively used for visual cues. Just ensure icons align with user expectations in the final UI.


28-34: Expose widget as a stable interface.
Naming RecentActivityWidget is appropriate. If this widget will be reused, consider adding memoization or lazy loading, depending on performance needs.


64-79: Empty states.
Returning <EmptyWorkspace /> when no recents appear is a clean approach. The scrollable container also helps manage large result sets, though you might want a “Load more” approach if data can become very large.

web/core/components/home/widgets/links/use-links.tsx (4)

30-52: Improve error messages for the create operation.
While the code adequately handles errors, consider adding more contextual information to help pinpoint failure causes, such as including the link’s title or user ID in the error message or logs.


53-71: Enforce consistent error data structure in the update method.
Here, you display a generic message and rethrow the error, whereas in the create operation you leverage error?.data?.error. Standardizing the response parsing increases clarity and consistency when debugging.


72-88: Add consistency to the remove error handling.
Similarly, to maintain uniformity, you might display the specific error message in addition to the toast for better debugging and user feedback.


93-97: Encourage unit testing for side effects and error handling.
The handleOnClose function is simple, yet it’s crucial to ensure that toggleLinkModal(false) is properly tested to avoid unexpected UI states. Consider adding dedicated test cases to validate these side effects.

Would you like help in crafting a sample test suite for this hook?

web/core/components/home/widgets/recents/issue.tsx (1)

23-83: Ensure robust fallback for missing issue data.
While the code checks for issueDetails and state, consider a fallback UI if issueDetails.name is undefined or an empty string. This avoids potential UI breakage in partial data scenarios.

web/core/components/home/widgets/links/link-detail.tsx (1)

30-30: Validate existence of linkDetail.
The guard clause returning an empty fragment is acceptable, but consider showing a placeholder or an EmptyState UI component if the link doesn’t exist. This keeps the user informed instead of silently rendering nothing.

web/core/components/home/widgets/empty-states/root.tsx (1)

87-116: Extract repeated layout logic into a reusable component.
The code that loops through EMPTY_STATE_DATA and renders each card is well-structured, but if future empty states are introduced in multiple places, consider factoring this out into a shared component to keep the code DRY.

web/core/components/home/widgets/links/create-update-link-modal.tsx (4)

31-33: Ensure consistent naming of props.
The prop naming is clear and succinct. Consider renaming props to a more descriptive name like modalProps for improved clarity.


49-58: Add user feedback upon creation/update failure.
While the modal logs an error to the console, it might be more user-friendly to display an error message in the UI to inform users that something went wrong.

} catch (error) {
  console.error("error", error);
+ // Optionally show a toast or error state in the UI
+ setErrorMessage("Failed to save your link. Please try again.")
}

73-96: Validate and autocomplete the URL.
You could leverage native attributes like type="url" or add RegExp-based validation for additional checks. This would enhance user experience and reduce invalid inputs.


123-129: Surface additional loading states.
When the parent state triggers a loading state or when there's a network request, you might consider disabling the cancel button to prevent conflicting operations.

<Button 
  variant="neutral-primary" 
  size="sm" 
  onClick={onClose}
+ disabled={isSubmitting}
>
  Cancel
</Button>
web/core/services/workspace.service.ts (3)

285-316: Handle detailed error responses on link operations.
For better debugging and user feedback, consider adding the HTTP status code or error response data to the thrown error instead of a broad error?.response.

.catch((error) => {
-  throw error?.response;
+  const statusCode = error?.response?.status;
+  const detail = error?.response?.data?.detail;
+  throw new Error(`Failed to process quick link request. Code: ${statusCode}, Detail: ${detail}`);
});

331-342: Add typed return value for fetchWorkspaceRecents.
The method is missing a specific return type. Add a type to help ensure the response structure is consistent throughout the codebase.

async fetchWorkspaceRecents(workspaceSlug: string, entity_name?: string): Promise<YourRecentsType[]> {
  return this.get(
    ...

344-363: Consider grouping widget methods into a dedicated widget service.
Widget fetching and updating logic may grow more complex over time. Isolate them in a dedicated file or class if usage becomes extensive, promoting cleaner architecture.

web/core/components/core/list/list-item.tsx (1)

66-66: Consider improving className string readability.

While the implementation is correct, the className string could be more readable by breaking it into multiple lines.

Consider this format for better readability:

-        <div className={cn("relative flex w-full items-center justify-between gap-3 overflow-hidden", itemClassName)}>
+        <div className={cn(
+          "relative flex w-full items-center justify-between gap-3 overflow-hidden",
+          itemClassName
+        )}>
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 790ecee and e9cd538.

📒 Files selected for processing (46)
  • packages/types/src/home.d.ts (1 hunks)
  • packages/types/src/index.d.ts (1 hunks)
  • web/app/[workspaceSlug]/(projects)/home/header.tsx (1 hunks)
  • web/app/[workspaceSlug]/(projects)/home/page.tsx (1 hunks)
  • web/ce/components/home/header.tsx (1 hunks)
  • web/ce/components/stickies/index.ts (1 hunks)
  • web/ce/components/stickies/widget.tsx (1 hunks)
  • web/core/components/core/list/list-item.tsx (3 hunks)
  • web/core/components/home/home-dashboard-widgets.tsx (1 hunks)
  • web/core/components/home/index.ts (1 hunks)
  • web/core/components/home/project-empty-state.tsx (1 hunks)
  • web/core/components/home/root.tsx (1 hunks)
  • web/core/components/home/user-greetings.tsx (1 hunks)
  • web/core/components/home/widgets/empty-states/index.ts (1 hunks)
  • web/core/components/home/widgets/empty-states/root.tsx (1 hunks)
  • web/core/components/home/widgets/index.ts (1 hunks)
  • web/core/components/home/widgets/links/action.tsx (1 hunks)
  • web/core/components/home/widgets/links/create-update-link-modal.tsx (1 hunks)
  • web/core/components/home/widgets/links/index.ts (1 hunks)
  • web/core/components/home/widgets/links/link-detail.tsx (1 hunks)
  • web/core/components/home/widgets/links/links.tsx (1 hunks)
  • web/core/components/home/widgets/links/root.tsx (1 hunks)
  • web/core/components/home/widgets/links/use-links.tsx (1 hunks)
  • web/core/components/home/widgets/loaders/index.ts (1 hunks)
  • web/core/components/home/widgets/loaders/loader.tsx (1 hunks)
  • web/core/components/home/widgets/loaders/quick-links.tsx (1 hunks)
  • web/core/components/home/widgets/loaders/recent-activity.tsx (1 hunks)
  • web/core/components/home/widgets/manage/index.tsx (1 hunks)
  • web/core/components/home/widgets/manage/widget-item-drag-handle.tsx (1 hunks)
  • web/core/components/home/widgets/manage/widget-item.tsx (1 hunks)
  • web/core/components/home/widgets/manage/widget-list.tsx (1 hunks)
  • web/core/components/home/widgets/manage/widget.helpers.ts (1 hunks)
  • web/core/components/home/widgets/recents/filters.tsx (1 hunks)
  • web/core/components/home/widgets/recents/index.tsx (1 hunks)
  • web/core/components/home/widgets/recents/issue.tsx (1 hunks)
  • web/core/components/home/widgets/recents/page.tsx (1 hunks)
  • web/core/components/home/widgets/recents/project.tsx (1 hunks)
  • web/core/components/page-views/workspace-dashboard.tsx (1 hunks)
  • web/core/hooks/store/use-home.ts (1 hunks)
  • web/core/hooks/use-workspace-issue-properties.ts (0 hunks)
  • web/core/layouts/auth-layout/workspace-wrapper.tsx (3 hunks)
  • web/core/services/workspace.service.ts (3 hunks)
  • web/core/store/workspace/home.ts (1 hunks)
  • web/core/store/workspace/index.ts (4 hunks)
  • web/core/store/workspace/link.store.ts (1 hunks)
  • web/ee/components/stickies/index.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • web/core/hooks/use-workspace-issue-properties.ts
✅ Files skipped from review due to trivial changes (10)
  • web/ce/components/stickies/index.ts
  • web/ce/components/home/header.tsx
  • web/ce/components/stickies/widget.tsx
  • web/core/components/home/widgets/loaders/index.ts
  • web/core/components/home/widgets/links/index.ts
  • web/core/components/home/widgets/empty-states/index.ts
  • web/core/components/home/widgets/index.ts
  • web/core/components/home/index.ts
  • web/ee/components/stickies/index.ts
  • web/core/components/page-views/workspace-dashboard.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/home/widgets/recents/page.tsx

[error] 32-38: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

web/core/components/home/root.tsx

[error] 68-93: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

🔇 Additional comments (42)
packages/types/src/index.d.ts (1)

41-41: No issues with the new export, but let's confirm the existence of the home module.

Everything looks correct for re-exporting the home module. However, to ensure there are no broken imports, let's confirm the module’s existence and check if it's being used properly.

✅ Verification successful

The home module exists and is properly structured

The module being exported (packages/types/src/home.d.ts) is present and contains valid TypeScript type declarations for home-related functionality. The export statement is correct.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `home` module exists and is exported correctly

# 1. Find the `home` files to ensure they exist in the repository.
fd home --exec ls "{}"

# 2. Search references to `home` export in the codebase to see usage.
rg 'from\s+"\.\/home"|from\s+\'\.\/home\''

Length of output: 555


Script:

#!/bin/bash
# Verify the content of home.d.ts and its usage
cat packages/types/src/home.d.ts

# Search for imports of the home module with fixed syntax
rg "from ['\"].*\/home['\"]" -g '*.ts' -g '*.tsx'

Length of output: 1783

web/core/store/workspace/link.store.ts (1)

89-93: Anticipate empty or partial service responses

The service call here could return null or an empty array. Offer fallback logic or error-handling to guard against unexpected data shapes.

web/core/store/workspace/home.ts (1)

92-125: Validate reordering logic for edge cases

The reordering logic uses sort_order to insert above or below. Consider boundary conditions (e.g., reordering the topmost or bottommost widget, or reassigning the same widget key).

web/core/components/home/widgets/manage/widget-item.tsx (2)

50-77: Detach event listeners to avoid memory leaks

You cleanly combine the draggable and dropTargetForElements in a useEffect return, which is great. Just ensure any additional event listeners you attach outside combine() are also properly cleaned up.


133-135: Ensure safe conversions to string

Calls to .toString() can fail if workspaceSlug is undefined. Double-check that workspaceSlug always has a valid string value before using it here.

web/core/hooks/store/use-home.ts (2)

1-6: Imports look consistent and concise.
No immediate concerns or duplicates, and the usage of the StoreContext alongside IHomeStore seems appropriate.


10-10: Validate non-null workspaceRoot.home.
Consider handling the scenario where workspaceRoot.home is unexpectedly undefined. A defensive check can prevent runtime errors in edge cases.

Would you like to see a script that searches your codebase for all references to workspaceRoot.home to ensure it's always initialized?

web/core/components/home/widgets/loaders/loader.tsx (2)

1-4: Imports established properly.
Loading the respective widget loaders is straightforward and neatly organized.


7-14: Props and enum definitions are clear.
Defining the EWidgetKeys enum and using it for the widgetKey prop is a good practice for maintaining type safety.

web/core/components/home/widgets/loaders/recent-activity.tsx (1)

1-20: Implementation looks correct and consistent.
This loader correctly generates multiple placeholder items for improved user perception during content loading. Using the item index as a key is acceptable here, as no dynamic reordering is expected.

web/core/components/home/widgets/manage/widget-item-drag-handle.tsx (1)

19-21: Good approach for drag cursor styling
The dynamic application of cursor classes based on the isDragging state is straightforward and follows intuitive UX patterns for draggable elements.

web/app/[workspaceSlug]/(projects)/home/page.tsx (1)

19-24: Layout is clear and easy to maintain
The usage of AppHeader, ContentWrapper, and PageHead yields a concise, readable structure. Good job!

web/core/components/home/widgets/manage/index.tsx (1)

29-30: Confirm the form behavior on “Save changes”
The button is of type "submit", but there is no wrapping form. If you intend to trigger a form submission, consider wrapping relevant fields in a <form> or changing it to type "button".

 <Button variant="primary" size="md" type="submit">
-  Save changes
+  Save changes
 </Button>
web/core/components/home/widgets/links/root.tsx (1)

34-37: Effective encapsulation of link management
Wrapping ProjectLinkList in a container that handles SWR fetching and a create/update modal fosters a well-structured, maintainable pattern for link management.

packages/types/src/home.d.ts (2)

4-4: Potential typo in filter key.

The string literal "all item" seems unintentional and potentially erroneous. It might be intended to be "all" or "all_items". Verify that related code handles this filter key correctly.


53-61: Validate date serialization.

created_at is typed as a Date. Confirm that relevant parts of the app (e.g., JSON requests/responses, database operations) properly serialize and deserialize date fields, especially if TLink is transmitted across the network.

web/app/[workspaceSlug]/(projects)/home/header.tsx (1)

3-59: Validate theming for GitHub logo.

Switching logos based on resolvedTheme is a good approach. Confirm fallback behavior if resolvedTheme is undefined or if the theme is not strictly "light"/"dark". For example, a newly integrated "system" or user-defined theme might break the logic.

web/core/components/home/user-greetings.tsx (2)

13-22: Consider adding fallback or error boundary for time formatting.

If user.user_timezone or currentTime is undefined or invalid, an error could occur during date formatting. Consider adding a quick fallback or error boundary in case any date/time-related data is missing or invalid.


41-47: Provide optional user name fallback.

When user?.first_name or user?.last_name are missing, the greeting line could display "undefined". Ensure there’s a fallback (e.g., an empty string or "User") to maintain a clean UI.

web/core/components/home/home-dashboard-widgets.tsx (2)

24-30: Guard against missing workspace slug.

You’re already returning null when workspaceSlug doesn’t exist. Confirm that this scenario is expected and tested, so you don’t end up hiding critical errors behind a silent null return.


41-45: Ensure comprehensive widget disable logic.

If isEnabled is false, the widget is omitted. Confirm there’s no scenario where critical fallback UI is needed (e.g., user tries to re-enable a widget but can’t). Possibly display a subtle placeholder if needed.

web/core/components/home/widgets/manage/widget-list.tsx (1)

15-20: Validate drop target.

The existence check for dropTargets.length > 0 is good, but also consider edge cases where the drag payload or the target data might be incomplete. A robust check ensures fewer runtime surprises.

web/core/components/home/widgets/recents/page.tsx (3)

1-9: Structure for uniform code.

Imports look well-organized, but confirm consistency in ordering across the project (e.g., external libs first, internal modules next). This fosters a standard structure, though it’s only necessary if enforced by the project’s guidelines.


40-43: Guard against null values.

pageDetails?.project_identifier and pageDetails?.name may be undefined. To avoid potential null or undefined output in the UI, ensure defaults or conditionals are in place.


57-59: Verify route structure for potential 404s.

Confirm that the path "/{workspaceSlug}/projects/{pageDetails?.project_id}/pages/{pageDetails.id}" is accurate. A missing or incorrect param could result in a broken route.

web/core/components/home/widgets/manage/widget.helpers.ts (2)

11-45: Validate edge cases in complex conditional logic.
The function’s conditional blocks handle multiple scenarios—blocked instructions, overlapping drop targets, etc. Before merging, ensure these edge cases are thoroughly tested (e.g., dropping a widget onto its own child, or no valid drop instruction). Overall, logic is readable.

Would you like me to provide test examples that cover corner cases (e.g., dropping within multiple nested groups) to ensure correctness?


1-3: Ensure consistent naming and usage of imports.
The imports and type references look proper. Just verify that @atlaskit/pragmatic-drag-and-drop-hitbox/tree-item remains stable, as external libraries might frequently change their exports.

Please run a quick search in the codebase to confirm that there are no conflicting or deprecated usages of this library:

✅ Verification successful

Imports are stable and consistently used across the codebase
The @atlaskit/pragmatic-drag-and-drop-hitbox package is properly versioned (1.0.3) and consistently used across all components. The imports follow the same patterns and there are no signs of deprecated or conflicting usage.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Searching for references to "pragmatic-drag-and-drop-hitbox" that might suggest a mismatch or conflict
rg 'pragmatic-drag-and-drop-hitbox'

Length of output: 2423

web/core/components/home/widgets/links/links.tsx (3)

1-9: Imports and dependencies.
All imports appear relevant and stable. MobX, React, and local references are being used correctly.


10-15: Re-check the exported types for consistency.
TLinkOperationsModal and TProjectLinkList seem clear in scope. Ensure they remain consistent with the rest of your codebase and do not conflict with similarly named types or interfaces in other modules.


17-28: Responsive design approach.
Using both MobX state and local states is a reasonable approach. The code consistently handles linkOperations and workspaceSlug. Looks good so far.

web/core/components/home/widgets/recents/project.tsx (2)

1-7: Imports verified.
All imported modules conform to expected usage. Dependencies are well-referenced.


13-19: Type casting approach.
Casting activity.entity_data to TProjectEntityData is acceptable if you’re certain about the entity type. Otherwise, ensure the code handles unexpected entity data gracefully (like runtime checks before casting).

web/core/components/home/widgets/recents/index.tsx (3)

1-8: Client entry.
Marking use client at the top ensures this code only runs on the client side. This is correct for SWR usage and fetching user data.


35-49: Validate reactivity handle on workspaceSlug.
Using workspaceSlug as a SWR key is a good approach. Double-check that changes to workspaceSlug are properly revalidated if route changes or workspace context changes mid-session.


51-62: Entity-specific rendering fallback.
resolveRecent covers recognized entities. For future expansions (e.g., new entity types), ensure a safe fallback to avoid blank UI or errors.

web/core/components/home/widgets/recents/issue.tsx (1)

9-13: Clarify ref usage in props.
Although “ref” is declared in the prop type, it might be unused or only passed to the parent. Confirm if you need the ref for scrolling or other DOM manipulations; if not, consider removing it from the props to reduce clutter.

✅ Verification successful

ref prop usage is verified and necessary
The ref prop is correctly used in the component, being passed to the ListItem component as parentRef. This is a common pattern for list virtualization and scroll management, making it a necessary prop that should be retained.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Checks references of the `ref` prop usage across the entire codebase.
rg -A 3 $'ref={' 

Length of output: 91


Script:

#!/bin/bash
# Find the component implementation
ast-grep --pattern 'const $_ = ({ activity, ref, workspaceSlug }: BlockProps) => {
  $$$
}'

# Backup search in case the component is defined differently
ast-grep --pattern 'function $_({ activity, ref, workspaceSlug }: BlockProps) {
  $$$
}'

# Also check if ref is used in any JSX elements in this file
cat "web/core/components/home/widgets/recents/issue.tsx"

Length of output: 3697

web/core/components/home/widgets/links/link-detail.tsx (1)

39-46: Confirm user feedback on copy actions.
The toast feedback is a nice addition. Ensure the copy operation works across all browsers or gracefully fails if clipboard APIs are unsupported.

✅ Verification successful

Clipboard implementation is robust and cross-browser compatible
The code uses a well-implemented clipboard strategy with proper fallback handling for older browsers and consistent user feedback through toast notifications.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Searches for fallback handling regarding clipboard. 
rg "navigator.clipboard" -A 5

Length of output: 4201


Script:

#!/bin/bash
# Search for fallback implementation and error handling
rg "fallbackCopyTextToClipboard" -A 10

Length of output: 3944

web/core/components/home/widgets/links/create-update-link-modal.tsx (1)

60-63: Avoid resetting to default values upon unmount if that is not desired behavior.
Currently, on unmount, the form resets to the default values. If you want to preserve the form values after closing, consider removing the reset call in the return callback.

useEffect(() => {
  if (isModalOpen) reset({ ...defaultValues, ...preloadedData });
- return () => reset(defaultValues);
+ return () => {
+   if (isModalOpen) {
+     reset(defaultValues);
+   }
+ };
}, [preloadedData, reset, isModalOpen]);
✅ Verification successful

Avoid unconditional form reset in cleanup as it's too broad
The current cleanup always resets the form to default values, even when the modal is already closed. This is overly aggressive and could lead to unwanted form resets. The suggested fix to only reset when the modal is open is the correct approach, as it properly matches the effect's setup condition.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for similar modal components with form handling
fd modal -e tsx -e ts | xargs rg -l "useEffect.*reset.*isModalOpen"

# Look for form reset patterns in cleanup functions
rg "return \(\) =>" -A 2 -B 2

# Check for custom form hooks
fd hook -e tsx -e ts | xargs rg -l "reset.*form"

Length of output: 30153

web/core/store/workspace/index.ts (1)

Line range hint 11-75: Ensure proper constructor dependency injection.
The HomeStore is instantiated without parameters, but it may require references to the rootStore or additional arguments for consistent data flow. Review whether it needs dependencies or if the stand-alone instantiation is intended.

this.home = new HomeStore();
+ // If needed:
+ // this.home = new HomeStore(_rootStore);
web/core/layouts/auth-layout/workspace-wrapper.tsx (1)

17-17: Confirm fetching of workspace states.
You are integrating fetchWorkspaceStates via SWR. Ensure that the logic for fetching is consistent across the store/hook to avoid partial or duplicated data fetch calls.

Also applies to: 51-51, 97-102

✅ Verification successful

Workspace states fetching implementation is consistent
The implementation follows a clean pattern with a single source of truth in ProjectStateService, proper state management via StateStore, and consistent usage across components. The SWR configuration in workspace-wrapper prevents unnecessary refetches, and there are no partial or duplicate fetch calls.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Checking all references to `fetchWorkspaceStates` to ensure each call is consistent
rg -A 5 "fetchWorkspaceStates"

Length of output: 2007


Script:

#!/bin/bash
# Check for any other state-fetching patterns
rg "getWorkspaceStates|workspaceStates" -A 3

Length of output: 2180

web/core/components/core/list/list-item.tsx (2)

21-21: LGTM! Clean interface extension.

The addition of the optional itemClassName property follows TypeScript best practices and maintains consistency with existing className props.


42-42: LGTM! Proper default value handling.

The default empty string for itemClassName prevents undefined className issues and maintains consistency with other styling props.

Comment on lines +106 to +115
updateLink = async (workspaceSlug: string, linkId: string, data: Partial<TLink>) => {
runInAction(() => {
Object.keys(data).forEach((key) => {
set(this.linkMap, [linkId, key], data[key as keyof TLink]);
});
});

const response = await this.workspaceService.updateWorkspaceLink(workspaceSlug, linkId, data);
return response;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider updating local state only after backend confirmation

In updateLink, you optimistically update linkMap before awaiting the service response. If the backend call fails, the store remains out of sync. Consider deferring local updates until after the service call completes successfully.

Comment on lines +117 to +127
removeLink = async (workspaceSlug: string, linkId: string) => {
// const issueLinkCount = this.getLinksByWorkspaceId(projectId)?.length ?? 0;
await this.workspaceService.deleteWorkspaceLink(workspaceSlug, linkId);

const linkIndex = this.links[workspaceSlug].findIndex((link) => link === linkId);
if (linkIndex >= 0)
runInAction(() => {
this.links[workspaceSlug].splice(linkIndex, 1);
delete this.linkMap[linkId];
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard against undefined references in removeLink

When removing a link, this.links[workspaceSlug] might be undefined if no links were previously loaded. Safeguard this access with a null check before calling findIndex.

-  const linkIndex = this.links[workspaceSlug].findIndex((link) => link === linkId);
+  const linkIds = this.links[workspaceSlug] || [];
+  const linkIndex = linkIds.findIndex((link) => link === linkId);
📝 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
removeLink = async (workspaceSlug: string, linkId: string) => {
// const issueLinkCount = this.getLinksByWorkspaceId(projectId)?.length ?? 0;
await this.workspaceService.deleteWorkspaceLink(workspaceSlug, linkId);
const linkIndex = this.links[workspaceSlug].findIndex((link) => link === linkId);
if (linkIndex >= 0)
runInAction(() => {
this.links[workspaceSlug].splice(linkIndex, 1);
delete this.linkMap[linkId];
});
};
removeLink = async (workspaceSlug: string, linkId: string) => {
// const issueLinkCount = this.getLinksByWorkspaceId(projectId)?.length ?? 0;
await this.workspaceService.deleteWorkspaceLink(workspaceSlug, linkId);
const linkIds = this.links[workspaceSlug] || [];
const linkIndex = linkIds.findIndex((link) => link === linkId);
if (linkIndex >= 0)
runInAction(() => {
this.links[workspaceSlug].splice(linkIndex, 1);
delete this.linkMap[linkId];
});
};

Comment on lines +7 to +9
export const useHome = (): IHomeStore => {
const context = useContext(StoreContext);
if (context === undefined) throw new Error("useDashboard must be used within StoreProvider");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Mismatch in error message.
The hook is named useHome, but the error message references useDashboard. This can cause confusion or mislead other developers.

Please update the error message accordingly:

- if (context === undefined) throw new Error("useDashboard must be used within StoreProvider");
+ if (context === undefined) throw new Error("useHome must be used within StoreProvider");
📝 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
export const useHome = (): IHomeStore => {
const context = useContext(StoreContext);
if (context === undefined) throw new Error("useDashboard must be used within StoreProvider");
export const useHome = (): IHomeStore => {
const context = useContext(StoreContext);
if (context === undefined) throw new Error("useHome must be used within StoreProvider");

Comment on lines +32 to +38
<>
{pageDetails?.logo_props?.in_use ? (
<Logo logo={pageDetails?.logo_props} size={16} type="lucide" />
) : (
<FileText className="h-4 w-4 text-custom-text-300" />
)}
</>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unnecessary React Fragment.

The static analysis tool’s hint is correct. Since there’s only one child element inside the Fragment, it can be removed.

- <>
-   {pageDetails?.logo_props?.in_use ? (
+ {pageDetails?.logo_props?.in_use ? (
      <Logo logo={pageDetails?.logo_props} size={16} type="lucide" />
    ) : (
      <FileText className="h-4 w-4 text-custom-text-300" />
    )}
- </>
📝 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
<>
{pageDetails?.logo_props?.in_use ? (
<Logo logo={pageDetails?.logo_props} size={16} type="lucide" />
) : (
<FileText className="h-4 w-4 text-custom-text-300" />
)}
</>
{pageDetails?.logo_props?.in_use ? (
<Logo logo={pageDetails?.logo_props} size={16} type="lucide" />
) : (
<FileText className="h-4 w-4 text-custom-text-300" />
)}
🧰 Tools
🪛 Biome (1.9.4)

[error] 32-38: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)

Comment on lines +47 to +62
/**
* This provides a boolean to indicate if the widget can be dropped onto the droptarget
* @param source
* @param widget
* @returns
*/
export const getCanDrop = (source: TDropTarget, widget: TWidgetEntityData | undefined) => {
const sourceData = source?.data;

if (!sourceData) return false;

// a widget cannot be dropped on to itself
if (sourceData.id === widget?.key) return false;

return true;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Confirm widget identity checks for drop validations.
getCanDrop prevents a widget from dropping onto itself, which is good. Consider whether additional constraints might be required (e.g., preventing a widget from dropping onto its descendants).

Comment on lines +49 to +74
const MENU_ITEMS: TContextMenuItem[] = [
{
key: "edit",
action: () => handleEdit(true),
title: "Edit",
icon: Pencil,
},
{
key: "open-new-tab",
action: handleOpenInNewTab,
title: "Open in new tab",
icon: ExternalLink,
},
{
key: "copy-link",
action: handleCopyText,
title: "Copy link",
icon: Link,
},
{
key: "delete",
action: () => linkOperations.remove(linkId),
title: "Delete",
icon: Trash2,
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enable undo or confirmation for destructive actions.
Deleting a link is irreversible in the UI. Consider adding a confirmation dialog or an “undo” feature to mitigate accidental deletions.

Comment on lines +21 to +85
const EMPTY_STATE_DATA = [
{
id: "create-project",
title: "Create a project",
description: "Create your first project now to get started",
icon: <Briefcase className="w-[40px] h-[40px] text-custom-primary-100" />,
cta: {
text: "Create Project",
onClick: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
if (!canCreateProject) return;
e.preventDefault();
e.stopPropagation();
setTrackElement("Sidebar");
toggleCreateProjectModal(true);
},
},
},
{
id: "invite-team",
title: "Invite your team",
description: "The sub text will be of two lines and that will be placed.",
icon: <Users className="w-[40px] h-[40px] text-custom-primary-100" />,
cta: {
text: "Invite now",
link: `/${workspaceSlug}/settings/members`,
},
},
{
id: "configure-workspace",
title: "Configure your workspace",
description: "The sub text will be of two lines and that will be placed.",
icon: <Hotel className="w-[40px] h-[40px] text-custom-primary-100" />,
cta: {
text: "Configure workspace",
link: "settings",
},
},
{
id: "personalize-account",
title: "Personalize your account",
description: "The sub text will be of two lines and that will be placed.",
icon:
currentUser?.avatar_url && currentUser?.avatar_url.trim() !== "" ? (
<Link href={`/${workspaceSlug}/profile/${currentUser?.id}`}>
<span className="relative flex h-6 w-6 items-center justify-center rounded-full p-4 capitalize text-white">
<img
src={getFileURL(currentUser?.avatar_url)}
className="absolute left-0 top-0 h-full w-full rounded-full object-cover"
alt={currentUser?.display_name || currentUser?.email}
/>
</span>
</Link>
) : (
<Link href={`/${workspaceSlug}/profile/${currentUser?.id}`}>
<span className="relative flex h-6 w-6 items-center justify-center rounded-full bg-gray-700 p-4 capitalize text-white text-sm">
{(currentUser?.email ?? currentUser?.display_name ?? "?")[0]}
</span>
</Link>
),
cta: {
text: "Personalize account",
link: "/profile",
},
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider user role constraints in “EMPTY_STATE_DATA.”
“Invite your team” and “Configure your workspace” might require admin rights. While you gate project creation with canCreateProject, consider also gating these actions.

@gakshita gakshita marked this pull request as draft January 6, 2025 16:16
@gakshita gakshita closed this Jan 7, 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.

3 participants