Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WEB-3291] dev: app sidebar revamp #6578

Merged
merged 21 commits into from
Feb 17, 2025
Merged

Conversation

anmolsinghbhatia
Copy link
Collaborator

@anmolsinghbhatia anmolsinghbhatia commented Feb 10, 2025

Description

This PR includes following changes:

  • App Sidebar: UI/UX improvements for better navigation.
  • Workspace section:
    • Pin/unpin and reorder navigation items.
  • Project section:
    • Project search and reorder functionality.
  • Profile Dropdown: Improved design and user interaction.
  • Code Refactor: Enhanced code maintainability and performance.

Type of Change

  • Feature
  • Improvement
  • Code refactoring

Screenshots and Media (if applicable)

Preview
preview

Test Scenarios

  • App Sidebar: Validate improved layout and navigation.
  • Workspace Sidebar: Test pin/unpin, reorder, and state persistence.
  • Project Sidebar: Verify search and reorder functionality.
  • Profile Dropdown: Check UI/UX improvements and actions.
  • General: Ensure no regressions, test on multiple devices and browsers.

References

[WEB-3291]

Summary by CodeRabbit

  • New Features

    • Enhanced user preference management with ordered settings for a more intuitive experience.
    • Introduced extended sidebar components with dynamic navigation, including drag-and-drop reordering, improved modal behavior, and enhanced responsiveness.
    • Added new sidebar navigation items and improved the structure for better user interaction.
    • Implemented new sidebar navigation preferences management, allowing fetching and updating of preferences.
  • Bug Fixes

    • Standardized key formats for sidebar navigation items to ensure consistent rendering.
  • Refactor

    • Streamlined the sidebar layout by reorganizing menu items and removing legacy project favorites for a cleaner and more consistent navigation experience.
    • Improved the handling of sidebar navigation preferences and state management.
    • Updated icon components for better styling and consistency.

Copy link
Contributor

coderabbitai bot commented Feb 10, 2025

Walkthrough

This pull request refines user preferences and sidebar navigation functionalities. Key changes include the removal of the "projects" key from user preferences, the addition of new keys, and enhancements to the response structure of user preferences. New TypeScript interfaces and constants have been introduced to support sidebar navigation. Several React components have been created or modified to implement extended sidebar features, including drag-and-drop support and dynamic sorting. Additionally, new hooks, service methods, and store properties have been added to manage sidebar preferences effectively.

Changes

File(s) Change Summary
apiserver/plane/app/views/workspace/user_preference.py
apiserver/plane/db/models/workspace.py
Updated the WorkspaceUserPreferenceViewSet to exclude the "projects" key, add a sort_order, rename variables, and alter the response structure; modified UserPreferenceKeys to remove PROJECTS and add ACTIVE_CYCLES, DRAFTS, and ARCHIVES.
packages/constants/src/workspace.ts
packages/types/src/workspace.d.ts
Added new TypeScript interfaces and constants for workspace sidebar navigation, including definitions for dynamic and static sidebar items and the new TPaginationInfo type.
web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx
web/app/[workspaceSlug]/(projects)/extended-sidebar.tsx
web/app/[workspaceSlug]/(projects)/sidebar.tsx
Introduced new React components (ExtendedProjectSidebar, ExtendedAppSidebar) and integrated them into the main sidebar; restructured sidebar content by removing deprecated components.
web/ce/components/workspace/sidebar/extended-sidebar-item.tsx
web/ce/components/workspace/sidebar/sidebar-item.tsx
web/ce/components/workspace/sidebar/helper.tsx
web/ce/components/workspace/sidebar/index.ts
Added a new CE component for extended sidebar items with drag-and-drop functionality along with helper functions and aggregated exports.
web/core/components/workspace/sidebar/dropdown.tsx
web/core/components/workspace/sidebar/projects-list-item.tsx
web/core/components/workspace/sidebar/projects-list.tsx
web/core/components/workspace/sidebar/sidebar-menu-items.tsx
web/core/components/workspace/sidebar/workspace-menu-item.tsx
Adjusted sidebar components by reordering elements, removing favorites logic, refining layouts, and updating key formats for consistency.
web/core/hooks/use-extended-sidebar-overview-outside-click.tsx Added a new custom hook for detecting clicks outside the extended sidebar to manage its collapse behavior.
web/core/layouts/auth-layout/workspace-wrapper.tsx
web/core/services/workspace.service.ts
web/core/store/theme.store.ts
web/core/store/workspace/index.ts
Enhanced fetching and updating of sidebar navigation preferences with new service methods, store observables, and actions, integrated into workspace authentication flow.
web/ee/components/workspace/sidebar/index.ts Re-exported all components from the CE sidebar module for easier access in the EE codebase.

Possibly related PRs

  • [WEB-3038]feat: home preferences #6308: The changes in the main PR regarding user preferences in the WorkspaceUserPreferenceViewSet class are related to the modifications in the WorkspacePreferenceViewSet class from the retrieved PR, as both involve handling user preferences, including sorting and managing preference keys.
  • feat: workspace user preference api #6497: The changes in the main PR regarding the WorkspaceUserPreferenceViewSet class and its handling of user preferences are directly related to the modifications in the retrieved PR, which also involves user preferences and the WorkspaceUserPreference model, specifically in the same context of managing user preferences.
  • feat: user preference url and sort order change #6505: The changes in the main PR are related to those in the retrieved PR as both involve modifications to the WorkspaceUserPreferenceViewSet class and the UserPreferenceKeys enumeration, specifically in how user preferences are handled and which keys are included or excluded.

Suggested labels

🌟improvement, 🛠️refactor

Suggested reviewers

  • SatishGandham

Poem

I’m a rabbit hopping through the code,
Skipping bugs on this winding road.
New sidebars bloom with ordered grace,
Drag and drop shines in its place.
Preferences set and stored anew,
My whiskers twitch in joy, it’s true!
Hop along, for every change is a delight!


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 14a8bd3 and 0344c17.

📒 Files selected for processing (1)
  • packages/ui/src/icons/overview-icon.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)
🔇 Additional comments (4)
packages/ui/src/icons/overview-icon.tsx (4)

5-5: LGTM! Props refactoring improves component flexibility.

The removal of hardcoded width/height defaults and the addition of rest props allows for better component reusability. The default className of "text-current" enables proper color inheritance.


6-6: LGTM! SVG element properly handles additional attributes.

The spread operator allows for flexible prop passing while maintaining the essential viewBox and className attributes.


8-9: LGTM! Attributes follow JSX naming conventions.

The camelCase attribute names properly align with React/JSX conventions.


11-11: LGTM! Dynamic color inheritance improves theme support.

Using currentColor instead of a hardcoded color value allows the icon to properly inherit colors from its parent element, supporting theming and dark mode.


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.

@anmolsinghbhatia anmolsinghbhatia marked this pull request as ready for review February 10, 2025 13:51
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: 5

🧹 Nitpick comments (23)
web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (4)

60-60: Avoid using as any for type casting.

Casting item.access as any may hide potential type mismatches or errors. Consider defining or refining the type for item.access to eliminate the need for an as any cast.


70-80: Consolidate pinning functionality.

pinNavigationItem and unPinNavigationItem share the same structure except for the boolean value. You could merge them into a single function (e.g. updatePinNavigationItem(isPinned: boolean)) to reduce duplication.

-const pinNavigationItem = (workspaceSlug: string, key: string) => {
-  updateSidebarPreference(workspaceSlug, key, {
-    is_pinned: true,
-  });
-};
-
-const unPinNavigationItem = (workspaceSlug: string, key: string) => {
-  updateSidebarPreference(workspaceSlug, key, {
-    is_pinned: false,
-  });
-};

+const updatePinNavigationItem = (workspaceSlug: string, key: string, pinned: boolean) => {
+  updateSidebarPreference(workspaceSlug, key, {
+    is_pinned: pinned,
+  });
+};

188-189: Add ARIA attributes for better accessibility.

The drag handle button could benefit from ARIA labels describing its purpose (e.g., aria-label="Drag to rearrange"). This ensures support for screen readers and assistive technologies.

<button
  ...
+  aria-label="Drag to rearrange"
>

206-219: Consider clarifying the pinned/unpinned icon actions.

Currently, the eye icon toggles pinned state in a short tooltip description (“Hide tab” vs. “Show tab”). For further clarity, verify if more descriptive text is preferable (e.g., “Unpin this tab” vs. “Pin this tab”), enhancing UX and discoverability.

web/ce/components/workspace/sidebar/helper.tsx (1)

5-26: Handle unmatched icon keys with a default case.

If none of the existing cases match, the function silently returns undefined. Consider adding a default return value (e.g., a generic icon or null) to avoid rendering issues.

  switch (key) {
    ...
    default:
+     return null;
  }
web/core/hooks/use-extended-sidebar-overview-outside-click.tsx (1)

9-38: Review nested condition checks to enhance readability.

The logic from lines 9-38 includes multiple early returns and nested loops. Consider refactoring for clarity, ensuring each condition is discoverable with minimal nesting.

web/core/components/workspace/sidebar/sidebar-menu-items.tsx (2)

21-21: Add type safety for workspaceSlug parameter

The workspaceSlug from useParams() could be undefined. Add type safety to prevent potential runtime errors.

-  const { workspaceSlug } = useParams();
+  const params = useParams();
+  const workspaceSlug = params?.workspaceSlug;
+  if (!workspaceSlug) return null;

29-39: Consider memoizing the sort function

The sort function could be memoized separately to prevent unnecessary recalculations.

+  const getSortOrder = useMemo(
+    () => (item: typeof WORKSPACE_SIDEBAR_DYNAMIC_NAVIGATION_ITEMS_LINKS[0]) => {
+      const preference = currentWorkspaceNavigationPreferences?.[item.key];
+      return preference?.sort_order ?? 0;
+    },
+    [currentWorkspaceNavigationPreferences]
+  );

   const sortedNavigationItems = useMemo(
     () =>
       WORKSPACE_SIDEBAR_DYNAMIC_NAVIGATION_ITEMS_LINKS.map((item) => ({
         ...item,
-        sort_order: preference ? preference.sort_order : 0,
+        sort_order: getSortOrder(item),
       })).sort((a, b) => a.sort_order - b.sort_order),
-    [currentWorkspaceNavigationPreferences]
+    [getSortOrder]
   );
web/core/components/workspace/sidebar/workspace-menu-item.tsx (1)

18-26: Improve type safety with literal types

Consider using literal types for the key property to prevent potential errors with key strings.

+type WorkspaceMenuItemKey = "active_cycles" | "home" | "notifications" | "projects";

 export type SidebarWorkspaceMenuItemProps = {
   item: {
     labelTranslationKey: string;
-    key: string;
+    key: WorkspaceMenuItemKey;
     href: string;
     Icon: React.ComponentType<any>;
     access: EUserWorkspaceRoles[];
   };
 };
apiserver/plane/app/views/workspace/user_preference.py (1)

42-45: Consider using a more maintainable sort order calculation

The current sort order calculation using magic numbers (65535 + (i*10000)) could be hard to maintain. Consider using named constants.

+BASE_SORT_ORDER = 65535
+SORT_ORDER_INCREMENT = 10000

 WorkspaceUserPreference(
-    key=key, user=request.user, workspace=workspace, sort_order=(65535 + (i*10000))
+    key=key, user=request.user, workspace=workspace, sort_order=(BASE_SORT_ORDER + (i * SORT_ORDER_INCREMENT))
 )
🧰 Tools
🪛 Ruff (0.8.2)

42-42: Line too long (107 > 88)

(E501)

web/ce/components/workspace/sidebar/sidebar-item.tsx (3)

77-77: Remove commented code

Remove the commented out code as it's not providing any value and could cause confusion.

-          {/* <icon className="size-4" /> */}

38-43: Consider extracting window width check to a custom hook

The window width check could be moved to a custom hook for better reusability and testability.

+const useIsMobileView = () => {
+  return typeof window !== 'undefined' && window.innerWidth < 768;
+};

 const handleLinkClick = () => {
-  if (window.innerWidth < 768) {
+  if (useIsMobileView()) {
     toggleSidebar();
   }
   if (extendedSidebarCollapsed) toggleExtendedSidebar();
 };

45-45: Consider moving static items to constants

The static items array should be moved outside the component to prevent recreation on each render.

+const STATIC_SIDEBAR_ITEMS = ["home", "notifications", "pi-chat", "projects"] as const;

 export const SidebarItem: FC<TSidebarItemProps> = observer((props) => {
-  const staticItems = ["home", "notifications", "pi-chat", "projects"];
web/app/[workspaceSlug]/(projects)/sidebar.tsx (1)

69-81: Consider extracting sidebar header into a separate component.

The sidebar header section containing workspace switcher, app switcher, and quick actions could be extracted into a reusable component to improve maintainability.

+import { SidebarHeader } from "@/components/workspace/sidebar/sidebar-header";

-          <div
-            className={cn("px-2", {
-              "px-4": !sidebarCollapsed,
-            })}
-          >
-            {/* Workspace switcher and settings */}
-            <SidebarDropdown />
-            <div className="flex-shrink-0 h-4" />
-            {/* App switcher */}
-            {canPerformWorkspaceMemberActions && <SidebarAppSwitcher />}
-            {/* Quick actions */}
-            <SidebarQuickActions />
-          </div>
+          <SidebarHeader 
+            sidebarCollapsed={sidebarCollapsed}
+            canPerformWorkspaceMemberActions={canPerformWorkspaceMemberActions}
+          />
web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (2)

66-70: Consider debouncing the project search.

The project filtering could benefit from debouncing to prevent excessive re-renders on each keystroke.

+import { useDebounce } from "@/hooks/use-debounce";

 const ExtendedProjectSidebar = observer(() => {
   const [searchQuery, setSearchQuery] = useState<string>("");
+  const debouncedSearchQuery = useDebounce(searchQuery, 300);
   
   const filteredProjects = joinedProjects.filter((projectId) => {
     const project = getPartialProjectById(projectId);
     if (!project) return false;
-    return project.name.toLowerCase().includes(searchQuery.toLowerCase()) || project.identifier.includes(searchQuery);
+    return project.name.toLowerCase().includes(debouncedSearchQuery.toLowerCase()) || project.identifier.includes(debouncedSearchQuery);
   });

143-143: Remove empty handleCopyText callback.

The handleCopyText prop is being passed an empty function. Either implement the copy functionality or remove it if unused.

-              handleCopyText={() => {}}
web/core/store/theme.store.ts (1)

76-87: LGTM! Well-documented toggle methods for extended sidebars.

The implementation follows the established pattern and includes proper JSDoc documentation.

However, consider initializing the state from local storage in the constructor to maintain the sidebar state across page reloads.

 constructor() {
+  // Initialize extended sidebar states from local storage
+  this.extendedSidebarCollapsed = localStorage.getItem("extended_sidebar_collapsed") === "true";
+  this.extendedProjectSidebarCollapsed = localStorage.getItem("extended_project_sidebar_collapsed") === "true";
+
   makeObservable(this, {

Also applies to: 89-100

packages/constants/src/workspace.ts (1)

261-298: Consider adding validation for navigation item keys.

While the implementation is good, consider adding runtime validation to ensure all required keys are present and valid.

// Add a validation function
const validateNavigationItem = (item: IWorkspaceSidebarNavigationItem): boolean => {
  return (
    typeof item.key === "string" &&
    typeof item.labelTranslationKey === "string" &&
    typeof item.href === "string" &&
    Array.isArray(item.access)
  );
};

// Use it when creating items
Object.values(WORKSPACE_SIDEBAR_DYNAMIC_NAVIGATION_ITEMS).forEach((item) => {
  if (!validateNavigationItem(item)) {
    throw new Error(`Invalid navigation item: ${item.key}`);
  }
});

Also applies to: 299-306

web/core/components/workspace/sidebar/dropdown.tsx (1)

205-266: Verify accessibility of the profile menu.

The profile menu implementation looks good, but consider adding the following accessibility improvements:

  • Add aria-label to the menu button
  • Add role="menuitem" to menu items
  • Consider adding keyboard navigation support
-        <Menu.Button className="grid place-items-center outline-none" ref={setReferenceElement}>
+        <Menu.Button 
+          className="grid place-items-center outline-none" 
+          ref={setReferenceElement}
+          aria-label="Open profile menu">
web/core/components/workspace/sidebar/projects-list.tsx (1)

282-295: Enhance "More" button accessibility.

The "More" button implementation is clean, but could benefit from improved accessibility.

-              className={cn("flex items-center gap-1.5 text-sm font-medium flex-grow text-custom-text-350", {
+              className={cn("flex items-center gap-1.5 text-sm font-medium flex-grow text-custom-text-350", {
                "justify-center": sidebarCollapsed,
               })}
+              aria-label="Show more projects"
+              aria-expanded="false"
web/core/services/workspace.service.ts (1)

375-385: Consider adding request validation.

The updateSidebarPreference method looks good, but consider adding validation for the key parameter.

   async updateSidebarPreference(
     workspaceSlug: string,
     key: string,
     data: Partial<IWorkspaceSidebarNavigationItem>
   ): Promise<IWorkspaceSidebarNavigationItem> {
+    if (!key || typeof key !== 'string') {
+      throw new Error('Invalid key parameter');
+    }
     return this.patch(`/api/workspaces/${workspaceSlug}/sidebar-preferences/${key}/`, data)
web/core/components/workspace/sidebar/projects-list-item.tsx (2)

92-190: Consider refactoring the drag-and-drop implementation for better maintainability.

While the implementation is functional, the drag-and-drop logic in the useEffect hook is quite complex. Consider:

  1. Extracting the drag-and-drop setup into a custom hook
  2. Breaking down the logic into smaller, focused functions
  3. Adding comments to explain the drag-and-drop behavior

Example refactor:

+// hooks/useDragAndDrop.ts
+const useDragAndDrop = ({
+  elementRef,
+  dragHandleRef,
+  projectId,
+  isLastChild,
+  projectListType,
+  handleOnProjectDrop,
+  disableDrag,
+  disableDrop,
+  isSidebarCollapsed
+}) => {
+  const [isDragging, setIsDragging] = useState(false);
+  const [instruction, setInstruction] = useState<"DRAG_OVER" | "DRAG_BELOW" | undefined>(undefined);
+
+  // Setup drag source
+  const getDragConfig = () => ({
+    element: elementRef.current,
+    canDrag: () => !disableDrag && !isSidebarCollapsed,
+    dragHandle: dragHandleRef.current ?? undefined,
+    getInitialData: () => ({ id: projectId, dragInstanceId: "PROJECTS" }),
+    onDragStart: () => setIsDragging(true),
+    onDrop: () => setIsDragging(false),
+    onGenerateDragPreview: // ... existing preview logic
+  });
+
+  // Setup drop target
+  const getDropConfig = () => ({
+    element: elementRef.current,
+    canDrop: ({ source }) =>
+      !disableDrop && source?.data?.id !== projectId && source?.data?.dragInstanceId === "PROJECTS",
+    getData: // ... existing getData logic,
+    onDrag: // ... existing onDrag logic,
+    onDragLeave: () => setInstruction(undefined),
+    onDrop: // ... existing onDrop logic
+  });
+
+  useEffect(() => {
+    const element = elementRef.current;
+    if (!element) return;
+
+    return combine(
+      draggable(getDragConfig()),
+      dropTargetForElements(getDropConfig())
+    );
+  }, [elementRef?.current, dragHandleRef?.current, projectId, isLastChild, projectListType, handleOnProjectDrop]);
+
+  return { isDragging, instruction };
+};
+
+// Component
-useEffect(() => {
-  // ... existing drag-and-drop logic
-}, [projectRef?.current, dragHandleRef?.current, projectId, isLastChild, projectListType, handleOnProjectDrop]);
+const { isDragging, instruction } = useDragAndDrop({
+  elementRef: projectRef,
+  dragHandleRef,
+  projectId,
+  isLastChild,
+  projectListType,
+  handleOnProjectDrop,
+  disableDrag,
+  disableDrop,
+  isSidebarCollapsed
+});
🧰 Tools
🪛 Biome (1.9.4)

[error] 173-173: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


301-315: Remove commented-out code for favorites functionality.

The commented-out code for favorites functionality should be removed since it's no longer needed, as indicated by the TODO comment.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between ce57c14 and c040a66.

📒 Files selected for processing (22)
  • apiserver/plane/app/views/workspace/user_preference.py (1 hunks)
  • apiserver/plane/db/models/workspace.py (1 hunks)
  • packages/constants/src/workspace.ts (2 hunks)
  • packages/types/src/workspace.d.ts (2 hunks)
  • web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (1 hunks)
  • web/app/[workspaceSlug]/(projects)/extended-sidebar.tsx (1 hunks)
  • web/app/[workspaceSlug]/(projects)/sidebar.tsx (3 hunks)
  • web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (1 hunks)
  • web/ce/components/workspace/sidebar/helper.tsx (1 hunks)
  • web/ce/components/workspace/sidebar/index.ts (1 hunks)
  • web/ce/components/workspace/sidebar/sidebar-item.tsx (1 hunks)
  • web/core/components/workspace/sidebar/dropdown.tsx (2 hunks)
  • web/core/components/workspace/sidebar/projects-list-item.tsx (3 hunks)
  • web/core/components/workspace/sidebar/projects-list.tsx (4 hunks)
  • web/core/components/workspace/sidebar/sidebar-menu-items.tsx (1 hunks)
  • web/core/components/workspace/sidebar/workspace-menu-item.tsx (1 hunks)
  • web/core/hooks/use-extended-sidebar-overview-outside-click.tsx (1 hunks)
  • web/core/layouts/auth-layout/workspace-wrapper.tsx (2 hunks)
  • web/core/services/workspace.service.ts (2 hunks)
  • web/core/store/theme.store.ts (6 hunks)
  • web/core/store/workspace/index.ts (7 hunks)
  • web/ee/components/workspace/sidebar/index.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • web/ee/components/workspace/sidebar/index.ts
  • web/ce/components/workspace/sidebar/index.ts
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/user_preference.py

42-42: Line too long (107 > 88)

(E501)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (16)
web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (1)

84-151: Review the drag-and-drop setup for potential side effects and code coverage.

  1. The useEffect dependencies might need consideration (e.g., ensuring no stale closures or referencing additional variables).
  2. Consider adding unit or integration tests for the drag-and-drop logic to confirm correct reordering, especially for edge cases like dropping on the last item.
apiserver/plane/app/views/workspace/user_preference.py (1)

54-61: Document the response format change

The change from list to dictionary response format is a breaking change that needs to be documented for API consumers.

❌ Verification inconclusive

🏁 Script executed:

#!/bin/bash
# Check if this change is documented in API docs
fd -t f "api.*md|readme.*md" -X grep -l "user preferences"

Length of output: 60


Document API Breaking Change for Response Format

Our search for documentation on "user preferences" did not return any results, which confirms that the API documentation does not reflect the breaking change from a list to a dictionary response. This change can impact API consumers, so please update the documentation accordingly.

  • Action Required: Update the API docs (e.g., README.md, API reference markdown files) to clearly document that the response format for user preferences has changed from a list to a dictionary.
web/app/[workspaceSlug]/(projects)/sidebar.tsx (1)

54-54: LGTM! Improved sidebar modularity with extended components.

The refactoring improves the sidebar's organization by splitting functionality into separate components: ExtendedAppSidebar and ExtendedProjectSidebar.

Also applies to: 109-111

web/app/[workspaceSlug]/(projects)/extended-sidebar.tsx (1)

27-37: LGTM! Efficient navigation items sorting with memoization.

The use of useMemo for sorting navigation items prevents unnecessary re-computations.

packages/types/src/workspace.d.ts (1)

227-235: LGTM! Well-structured type definitions for sidebar navigation.

The interfaces IWorkspaceSidebarNavigationItem and IWorkspaceSidebarNavigation provide clear type safety for the sidebar navigation features.

web/core/store/theme.store.ts (1)

6-7: LGTM! New observable properties for extended sidebars.

The new properties follow the same pattern as existing ones, maintaining consistency in the codebase.

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

233-235: LGTM! Efficient use of computedFn.

Using computedFn from mobx-utils is a good choice here as it memoizes the computed value based on the input parameter.

packages/constants/src/workspace.ts (1)

254-259: LGTM! Well-defined interface for sidebar navigation items.

The interface is clear and includes all necessary properties for navigation items.

web/core/layouts/auth-layout/workspace-wrapper.tsx (1)

104-109: LGTM! Consistent implementation of sidebar preferences fetching.

The implementation follows the established pattern of using SWR for data fetching with appropriate revalidation options.

web/core/components/workspace/sidebar/dropdown.tsx (1)

82-86: Improved responsive layout with conditional flex direction.

The updated container styling enhances the responsive behavior by using conditional classes based on the sidebar state.

web/core/components/workspace/sidebar/projects-list.tsx (2)

8-8: LGTM! Clean dependency management.

The addition of Ellipsis icon and toggleExtendedProjectSidebar hook is well-organized and follows best practices.

Also applies to: 41-41


241-246: Consider pagination implications.

Limiting the displayed projects to 7 improves performance and UI cleanliness. However, ensure that the user experience remains smooth when switching between the main list and extended sidebar.

Run this script to check the average number of projects per workspace:

❌ Verification inconclusive

🏁 Script executed:

#!/bin/bash
# Description: Check project distribution across workspaces
ast-grep --pattern 'class Project {
  $$$
  workspace = models.ForeignKey($$$)
  $$$
}'

Length of output: 89


Pagination & User Experience: Further Verification Required

The review comment advises ensuring that limiting the displayed projects to 7 does not negatively impact user experience—especially when toggling to an extended list—by considering potential pagination or overflow issues. Our initial attempt to gather evidence (via the provided ast-grep query) returned no results, so this aspect remains inconclusive.

  • Action Needed: Manually verify that the typical number of projects per workspace (and their distribution) aligns with displaying only 7 projects without degrading the UX when switching between the main list and the extended sidebar.
  • Recommendation: Confirm whether the current slicing logic adequately supports users who have more than 7 projects. It might be helpful to review related UI/UX flows or test with sample data to ensure smooth interaction.
web/core/services/workspace.service.ts (1)

367-373: LGTM! Well-structured API method.

The fetchSidebarNavigationPreferences method follows the established pattern for API calls with proper error handling.

web/core/components/workspace/sidebar/projects-list-item.tsx (3)

3-46: LGTM! Well-structured imports and type definitions.

The imports and type definitions are properly organized, with clear separation between external libraries, UI components, and internal utilities. The Props interface is well-documented and includes all necessary properties for the drag-and-drop functionality.


48-86: LGTM! Well-implemented state management and permission checks.

The component follows React best practices with:

  • Proper use of MobX for state management
  • Clear separation of concerns
  • Comprehensive permission checks for different user roles

191-402: LGTM! Well-structured render logic with proper accessibility.

The component implements proper accessibility using Headless UI components (Disclosure, Transition) and follows React best practices for conditional rendering.

web/core/store/workspace/index.ts Show resolved Hide resolved
web/core/store/workspace/index.ts Show resolved Hide resolved
apiserver/plane/db/models/workspace.py Show resolved Hide resolved
@gakshita
Copy link
Collaborator

Remove blue focus border from sidebar items on focus

@gakshita
Copy link
Collaborator

Sorting not working for drafts and archives

@anmolsinghbhatia anmolsinghbhatia marked this pull request as draft February 11, 2025 09:47
@anmolsinghbhatia
Copy link
Collaborator Author

Sorting not working for drafts and archives

@gakshita It’s working for me. Can you share steps to replicate it?

@anmolsinghbhatia
Copy link
Collaborator Author

Remove blue focus border from sidebar items on focus

Resolved: Removed the blue focus border from extended sidebar icon on focus.

@anmolsinghbhatia anmolsinghbhatia marked this pull request as ready for review February 13, 2025 07:10
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: 0

🧹 Nitpick comments (5)
web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (5)

42-42: Fix typo in ref name.

The ref variable name has a typo: navigationIemRef should be navigationItemRef.

-  const navigationIemRef = useRef<HTMLDivElement | null>(null);
+  const navigationItemRef = useRef<HTMLDivElement | null>(null);

40-40: Consider using a union type for instruction state.

Define a union type for better type safety and code maintainability.

+  type TDragInstruction = "DRAG_OVER" | "DRAG_BELOW";
-  const [instruction, setInstruction] = useState<"DRAG_OVER" | "DRAG_BELOW" | undefined>(undefined);
+  const [instruction, setInstruction] = useState<TDragInstruction | undefined>(undefined);

64-68: Consider making href construction more robust.

The current href construction could be improved to handle edge cases and ensure consistency.

-  const itemHref =
-    item.key === "your_work"
-      ? `/${workspaceSlug.toString()}${item.href}${data?.id}`
-      : `/${workspaceSlug.toString()}${item.href}`;
+  const itemHref = (() => {
+    const baseUrl = `/${workspaceSlug.toString()}`;
+    if (!item.href) return baseUrl;
+    const cleanHref = item.href.startsWith('/') ? item.href : `/${item.href}`;
+    return item.key === "your_work" && data?.id
+      ? `${baseUrl}${cleanHref}${data.id}`
+      : `${baseUrl}${cleanHref}`;
+  })();

91-91: Extract magic string to a constant.

The "NAVIGATION" string is used as a drag instance identifier and should be defined as a constant.

+const DRAG_INSTANCE_ID = "NAVIGATION" as const;
// ...
-        getInitialData: () => ({ id: item.key, dragInstanceId: "NAVIGATION" }),
+        getInitialData: () => ({ id: item.key, dragInstanceId: DRAG_INSTANCE_ID }),

157-159: Consider extracting complex className combinations.

The className combinations could be more maintainable by extracting them into constants or utility functions.

+  const getItemClassName = (isDragging: boolean) => cn(
+    "group/project-item relative w-full flex items-center rounded-md",
+    "text-custom-sidebar-text-100 hover:bg-custom-sidebar-background-90"
+  );
// ...
-        className={cn(
-          "group/project-item relative w-full  flex items-center rounded-md text-custom-sidebar-text-100 hover:bg-custom-sidebar-background-90"
-        )}
+        className={getItemClassName(isDragging)}
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between c040a66 and 63c7404.

📒 Files selected for processing (4)
  • packages/hooks/src/use-outside-click-detector.tsx (1 hunks)
  • web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (1 hunks)
  • web/core/components/workspace/sidebar/dropdown.tsx (2 hunks)
  • web/core/hooks/use-extended-sidebar-overview-outside-click.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/hooks/src/use-outside-click-detector.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/core/hooks/use-extended-sidebar-overview-outside-click.tsx
  • web/core/components/workspace/sidebar/dropdown.tsx
🔇 Additional comments (2)
web/ce/components/workspace/sidebar/extended-sidebar-item.tsx (2)

23-33: LGTM! Well-structured type definition.

The TExtendedSidebarItemProps type is well-defined with clear prop types and optional parameters.


35-220: Overall implementation looks great!

The ExtendedSidebarItem component is well-implemented with proper drag-and-drop support, accessibility features, and clean separation of concerns. The component aligns perfectly with the PR objectives for the sidebar revamp.

@sriramveeraghanta sriramveeraghanta modified the milestones: v0.14.3-dev, v1.0.0 Feb 14, 2025
@anmolsinghbhatia anmolsinghbhatia marked this pull request as draft February 17, 2025 06:20
@anmolsinghbhatia anmolsinghbhatia marked this pull request as ready for review February 17, 2025 15:38
@sriramveeraghanta sriramveeraghanta merged commit 473932a into preview Feb 17, 2025
5 of 6 checks passed
@sriramveeraghanta sriramveeraghanta deleted the dev-app-sidebar-revamp branch February 17, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟enhancement New feature or request 🌐frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants