[WEB-2846] feat: home integrations #6321
Conversation
WalkthroughThis pull request introduces comprehensive enhancements to the workspace dashboard functionality across multiple components in the web application. The changes span frontend components, TypeScript type definitions, API serializers, and store management. The primary focus is on implementing a more dynamic and interactive home dashboard with features like quick links, recent activities, widget management, and improved state handling. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
gakshita
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (46)
web/core/store/workspace/home.ts (3)
8-19: Ensure consistent annotation and expand doc comments.
TheIHomeStoreinterface effectively documents your observable fields and actions. Consider adding doc comments for each property and function, especially describing any assumptions about parameters or return values, so future developers can quickly grasp usage.
30-48: Consider passing dependencies to the constructor.
TheHomeStoreconstructor initializes theWorkspaceServiceandWorkspaceLinkStoreinternally. Injecting these as dependencies can make the code more testable and decouple the store from specific service implementations.
54-67: Handle user-friendly error feedback.
Here, an error is logged and rethrown, which is acceptable for debugging. However, end-users might need a friendly notification. Consider adding a user-facing error message or dispatching a custom error event.web/core/components/home/widgets/manage/widget-item.tsx (3)
27-31: Use more specific type definitions for props.
widget: anymay obscure the data structure and hamper type-checking. Consider defining a dedicated interface or type for your widget to improve safety and maintainability.
74-92: Expose clearer event structure from onDrag & onDrop.
InonDragandonDrop, you interpret and attach instructions to the payload. If other developers extend or override this logic, providing typed event objects or clarifying docs could prevent confusion.
109-129: ToggleSwitch usage is commented out.
Given the placeholders ({/* <ToggleSwitch /> */}), ensure this is intentional. If the toggle was part of the design, consider adding it or removing the comment to avoid confusion.web/core/components/home/widgets/index.ts (1)
1-4: Avoid duplicate export of the same module.
You’re exporting"./empty-states"twice. Removing line 4 will maintain the same behavior without the redundant export.1 export * from "./empty-states"; 2 export * from "./loaders"; 3 export * from "./recents"; -4 export * from "./empty-states";web/core/components/home/widgets/loaders/recent-activity.tsx (1)
7-20: Optional parameter for placeholder items.
Currently,range(7)is hardcoded. Consider exposing this as a prop for configurable placeholders, which could make the loader more flexible.-export const RecentActivityWidgetLoader = () => ( - <Loader className="bg-custom-background-100 rounded-xl px-2 space-y-6"> - {range(7).map((index) => ( +interface RecentActivityWidgetLoaderProps { + count?: number; +} + +export const RecentActivityWidgetLoader = ({ count = 7 }: RecentActivityWidgetLoaderProps) => ( + <Loader className="bg-custom-background-100 rounded-xl px-2 space-y-6"> + {range(count).map((index) => (web/core/components/home/widgets/links/action.tsx (1)
10-18: Improve accessibility with aria-label.
For better screen reader support, consider adding anaria-labelor a more descriptive text for the button.<button ... + aria-label="Add quick link" onClick={onClick} >web/app/[workspaceSlug]/(projects)/home/page.tsx (1)
12-16: Fallback if workspace not found.
pageTitlebecomesundefinedifcurrentWorkspacedoes not exist. Consider providing a default value or gracefully handling a missing workspace scenario.-const pageTitle = currentWorkspace?.name ? `${currentWorkspace?.name} - Home` : undefined; +const pageTitle = currentWorkspace?.name + ? `${currentWorkspace.name} - Home` + : "Home";web/core/components/home/widgets/manage/index.tsx (2)
3-4: Consider removing unused imports
It appears thatmobx-react'sobservermight be the only needed import. Remove or consolidate any unused imports to keep the file tidy and maintainable.
16-19: Prop destructuring for better readability
Destructuring props on line 18 clarifies usage, but consider destructuring at the function signature level for improved clarity:-export const ManageWidgetsModal: FC<TProps> = observer((props) => { - const { workspaceSlug, isModalOpen, handleOnClose } = props; +export const ManageWidgetsModal: FC<TProps> = observer(({ workspaceSlug, isModalOpen, handleOnClose }) => {packages/types/src/index.d.ts (1)
41-41: Validate the appropriateness of wildcard exports
Ensure that re-exporting everything from"./home"is intentional. Overly broad exports can sometimes leak internal types or cause name collisions. If certain types aren't needed elsewhere, consider named exports for greater control.web/core/components/home/widgets/links/root.tsx (2)
1-2: Remove unused imports if not required
Confirm ifmobx-reactandswrare both essential here. If certain parts of the import are unused, consider removing them to keep the file clutter-free.
34-37: Responsive layout considerations
Using a fixed border and padding is fine, but ensure that the.flexlayout withmx-autoandw-fullmeets all screen-size requirements. Check if content resizes properly for small or large displays.web/core/components/home/widgets/manage/widget-list.tsx (2)
10-14: Consider dynamic widget definitions
The hardcodedWIDGETS_LISTis simple, but plan how to handle dynamic or user-configurable widgets in the future. Keeping it static may limit extensibility as user requirements grow.
37-47: Add meaningful keys or local IDs for better trackability
Currently, the map key useswidget.id. Confirm eachwidget.idis unique or if it needs a more distinctive identifier for advanced usage—especially if widget definitions become dynamic.web/core/components/home/project-empty-state.tsx (1)
30-45: Consider alternative empty states or fallback actions.The empty state is well-designed, but consider guiding users who can't create projects (e.g., read-only members) toward exploring other workspace features or requesting permissions from an admin.
web/core/components/home/home-dashboard-widgets.tsx (2)
28-28: Provide a fallback UI whenworkspaceSlugis missing.Returning
nullmay cause a confusing experience for users. Consider providing a user-friendly message or redirecting them if the workspaceSlug is undefined.
39-49: Preserve a predictable widget ordering.Using
Object.entries(WIDGETS_LIST).mapmay produce an order dependent on object-key iteration. If a strict order is required for UX consistency, consider using an array or a separately maintained order.web/core/components/home/widgets/recents/filters.tsx (2)
30-30: Confirm pluralization usage.Appending an “s” to the filter name can produce awkward labels (e.g.,
"all items"vs."all itemss"). Consider maintaining a separate display label to avoid incorrect pluralizations.
40-43: Maintain consistent naming and grammar in the button label.Similar to the previous block, ensure filters like
"page"become"pages","project"becomes"projects", and so forth in a user-friendly manner rather than relying solely onfilter.name + "s".web/core/components/home/widgets/recents/page.tsx (1)
25-61: Refactor to remove unnecessary Fragment.
At lines 32–38, wrapping the ternary operator in a fragment is unnecessary. You can safely eliminate the fragment:- <> - {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)
web/core/components/home/widgets/links/links.tsx (2)
30-52: Resize listener approach is fine, but consider debouncing.
Frequent calls tosetColumnCounton everyresizeevent can impact performance on resize-intensive scenarios or older devices. Debouncing the handler would optimize performance without losing responsiveness.+ let resizeTimer: NodeJS.Timeout | null = null; const updateColumnCount = () => { + if (resizeTimer) clearTimeout(resizeTimer); + resizeTimer = setTimeout(() => { if (window.matchMedia("(min-width: 1024px)").matches) { setColumnCount(4); } else if (window.matchMedia("(min-width: 768px)").matches) { setColumnCount(3); } else if (window.matchMedia("(min-width: 640px)").matches) { setColumnCount(2); } else { setColumnCount(1); } + }, 150); };
53-63: Consider fallback UI for empty links array.
Currently, you show a loader for undefined links. Iflinksis defined but empty, the user sees no direct visual feedback. Providing a simple message or placeholder for zero links can help clarity.web/core/components/home/widgets/recents/project.tsx (1)
37-59: MemberDropdown display logic is correct.
Conditionally rendering the dropdown only if members exist is good for usability. Ensure that thez-10stacking context does not conflict with other modals.web/core/components/home/widgets/recents/index.tsx (3)
21-26: Static array of filters is concise and user-friendly.
The presence of icons helps guide users visually. Consider localizing filter names if the application supports multiple languages.
35-49: SWR usage is correct, but consider error handling.
You handleisLoadingeffectively. Ifrecentsis null due to network errors, you might want to show an error or fallback UI instead of an empty state or indefinite loading.
73-77: Wrapping items with an overflow container.
Limiting the max height and making the content scrollable helps design consistency. Validate that 500px is an appropriate fixed limit for all screen sizes.web/core/components/home/widgets/links/use-links.tsx (2)
35-35: Consider removing or replacing console logs.You have a
console.logstatement that might be left over from debugging. Removing it or converting it to a proper logger can help avoid leaking debug information.- console.log("data", data, workspaceSlug);
72-88: Await error handling or rethrow for consistency.Currently, when removing a link fails, you set a toast to indicate an error but do not rethrow the error as done in the other operations. Consider throwing the error or handling it consistently across all operations.
-remove: async (linkId: string) => { +remove: async (linkId: string): Promise<void> => { try { if (!workspaceSlug) throw new Error("Missing required fields"); await removeLink(workspaceSlug, linkId); setToast({ // ... }); } catch (error) { setToast({ // ... }); + throw error; } },web/core/components/home/root.tsx (1)
67-92: Remove the unnecessary fragment.The nested fragment wrapping lines 67–92 is not needed. You can remove it and return the content directly to reduce complexity, matching the static analysis hint.
{homeDashboardId && joinedProjectIds && ( - <> {joinedProjectIds.length > 0 || loader ? ( <> <IssuePeekOverview /> <ContentWrapper ... /> </> ) : ( <EmptyState ... /> )} - </> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 67-92: 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/widgets/links/link-detail.tsx (3)
30-30: Return null instead of an empty fragment for clarity.When no
linkDetailis found, returningnullis more conventional than an empty fragment, making the intent clearer.if (!linkDetail) - return <></>; + return null;
47-47: Enhance security when opening links.When opening external links with
window.open, consider specifyingrel="noopener noreferrer"to mitigate security risks like reverse tabnabbing.- const handleOpenInNewTab = () => window.open(`${viewLink}`, "_blank"); + const handleOpenInNewTab = () => window.open(`${viewLink}`, "_blank", "noopener,noreferrer");
70-70: Await the link removal for consistency.All other operations await their promises. Consider awaiting the
removemethod for better consistency and error handling.{ key: "delete", - action: () => linkOperations.remove(linkId), + action: async () => { + await linkOperations.remove(linkId); + }, title: "Delete", icon: Trash2, },web/core/store/workspace/link.store.ts (3)
82-87: Potential simplification oflodash/setusage.The call to
set(this.linkMap, link.id, link)can be replaced with direct indexing, ensuring more intuitive code and avoiding an external dependency in this case:- links.forEach((link) => set(this.linkMap, link.id, link)); + links.forEach((link) => { + this.linkMap[link.id] = link; + });
95-96: Remove or refine debugging logs in production code.
console.log("hereee");appears to be a debugging artifact. Consider removing or replacing with a more descriptive log message to maintain clean and purposeful output.- console.log("hereee"); + // console.debug("Creating a workspace link - for debugging only");
117-119: Unused commented-out code.The commented-out line references
issueLinkCount, but there's no usage in the final implementation. If no longer needed, consider removing it to keep the codebase clean.- // const issueLinkCount = this.getLinksByWorkspaceId(projectId)?.length ?? 0;web/core/components/home/widgets/empty-states/root.tsx (2)
1-84: Consider adding accessibility labels for icons and CTA elements.The icons and CTA elements in the
EMPTY_STATE_DATAarray (e.g., "Invite your team") could benefit fromaria-labelortitleattributes for improved accessibility. This will help screen readers describe these elements more effectively.
59-79: Potential i18n support for displayed text.All text strings (e.g., "Personalize your account") are hard-coded. If internationalization is a requirement, consider encapsulating these strings using a translation layer for future multilingual support.
web/core/store/workspace/index.ts (1)
46-46: Ensure store dependencies are properly passed if needed.Currently,
this.home = new HomeStore();is created without passing_rootStore. IfHomeStoreeventually needs root store context, ensure it is updated accordingly. For example,new HomeStore(_rootStore).web/core/layouts/auth-layout/workspace-wrapper.tsx (1)
97-102: SWR invocation for workspace states looks consistent.
The pattern matches existing calls (e.g.,fetchProjects,fetchFavorite). Ensure you handle any potential rejection scenarios withinfetchWorkspaceStatesso that errors surface properly in this component.web/core/services/workspace.service.ts (3)
285-316: Quick links methods.
These methods provide CRUD for quick links. Consider adding more specific error handling or message logs to help debug failures, as each.catch()block only rethrows the response.
331-342: Workspace recents method.
Fetching workspace recents without pagination can be fine for moderate usage, but if the list grows, we might face performance issues. Keep an eye on lengths or possibly add pagination in the future.
344-363: Widgets endpoints.
The new widget methods align well with existing patterns. If you expect multiple widget categories in the future, consider structuring them with more specialized endpoints or query parameters.apiserver/plane/app/serializers/workspace.py (1)
149-149: IncludeidinIssueRecentVisitSerializer.
Adding theidfield helps maintain a consistent data shape in the front end. The line is a bit long per style guidelines, so consider breaking it up if your code style enforces max line length.- fields = ["id", "name", "state", "priority", "assignees", "type", "sequence_id", "project_id", "project_identifier", ] + fields = [ + "id", + "name", + "state", + "priority", + "assignees", + "type", + "sequence_id", + "project_id", + "project_identifier", + ]🧰 Tools
🪛 Ruff (0.8.2)
149-149: Line too long (134 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
apiserver/plane/app/serializers/workspace.py(2 hunks)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 (9)
- web/core/components/home/widgets/empty-states/index.ts
- web/ce/components/stickies/widget.tsx
- web/ce/components/home/header.tsx
- web/ee/components/stickies/index.ts
- web/core/components/home/widgets/links/index.ts
- web/core/components/page-views/workspace-dashboard.tsx
- web/core/components/home/widgets/loaders/index.ts
- web/core/components/home/index.ts
- web/ce/components/stickies/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/home/root.tsx
[error] 67-92: 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/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)
🪛 Ruff (0.8.2)
apiserver/plane/app/serializers/workspace.py
149-149: Line too long (134 > 88)
(E501)
164-164: Line too long (98 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint-web
- GitHub Check: lint-space
- GitHub Check: lint-admin
- GitHub Check: Analyze (javascript)
🔇 Additional comments (60)
web/core/store/workspace/home.ts (1)
69-81: Verify concurrency or conflicts in widget toggle.
When toggling a widget, ensure no conflicts can arise if another user or process toggles it simultaneously on the backend. If concurrency is possible, verify that the finalis_enabledstate remains correct after subsequent toggles.web/core/components/home/widgets/manage/widget-item.tsx (2)
3-14: Code organization is clear, but watch out for library changes.
Imports from@atlaskit/pragmatic-drag-and-droprely on internal types and submodule paths (dist/types/internal-types). Atlassian may change these internals without warning. Keep an eye on library updates to avoid potential breaks.
43-70: Double-check performance implications of the drag preview.
Rendering a custom drag preview is excellent for UX, but repeated calls torenderondragstartcan be expensive if used heavily. Confirm that the cost of mounting/unmounting the preview is not excessive.web/core/components/home/widgets/loaders/quick-links.tsx (1)
1-13: Optimize the range generation if dynamic counts are large.
Currently, generating four loader items withrange(4)is fast. If future changes demand many placeholders,Array.from({ length: n })or a custom approach could offer more flexibility. Right now, for four items, this is acceptable.web/core/components/home/widgets/loaders/recent-activity.tsx (1)
1-2: "use client" directive confirmed.
The directive is appropriate here for a client-only component.web/core/components/home/widgets/links/action.tsx (1)
1-5: Type definition is concise.
TheTPropsdefinition foronClickis clear and straightforward.web/core/components/home/widgets/manage/widget-item-drag-handle.tsx (2)
14-26: Dragging style is well-implemented.
The conditional class application usingcnforcursor-grabbingis a neat approach. No issues found.
9-12: Unused prop:sort_order.
Thesort_orderprop is not used. Confirm if it’s needed or remove it to keep the component minimal.web/app/[workspaceSlug]/(projects)/home/page.tsx (1)
17-25: Overall structure.
The layout structure withAppHeader,ContentWrapper, and the customPageHeadis coherent and follows a clean pattern.web/core/components/home/widgets/manage/index.tsx (1)
20-35: Finalize "Save changes" click behavior
The "Save changes" button currently has no explicit handler. If you intend to persist widget settings, wire up anonClickor form submission to complete the action. Otherwise, rename it to something that indicates no additional steps are taken.web/core/components/home/widgets/links/root.tsx (1)
16-24: Validate SWR revalidation strategy
You have disabled revalidation on focus and reconnect. Ensure this is a deliberate decision—if data freshness is crucial, these flags may need to be updated to allow real-time updates.web/core/components/home/widgets/manage/widget-list.tsx (1)
18-35: Confirm drag-and-drop reorder logic
ThehandleDropcode reorders widgets based onsourceData.idanddroppedId. Ensure edge cases are covered, such as dropping a widget onto itself or dropping outside valid targets.packages/types/src/home.d.ts (2)
53-61: Avoid usinganyformetadataand reconsidercreated_atasDate.Using
anyis generally discouraged in strongly typed systems. Also, ifcreated_atis coming from APIs as a string, storing it directly asDatemay cause serialization issues. Consider using a more specific type formetadataor storingcreated_atas a string or a union type (string | Date) if needed.
71-76: LGTM on widget entity definitions.The fields for
TWidgetEntityDataare clear and well-defined.web/core/components/home/project-empty-state.tsx (1)
19-21: Ensure robust permission handling.
canCreateProjectis determined viaallowPermissions(...)and looks good for controlling UI. However, ensure the server-side also prevents unauthorized creation of projects to maintain security consistency.web/app/[workspaceSlug]/(projects)/home/header.tsx (3)
1-2: Use client directive looks good.
No issues found with the"use client"directive.
3-16: Imports and Hooks are well-structured.
The import statements are concise, and the usage ofuseThemeand custom hooks is consistent with best practices.
18-60: Header logic and markup are correct.
The overall structure, event tracking, and conditional image rendering for GitHub's logo based on theme are implemented well. Accessibility attributes for external links are also used correctly.web/core/components/home/user-greetings.tsx (10)
1-2: Imports and file setup appear correct.
No issues.
3-5: Hook imports are appropriate.
Usage oflucide-reacticons,IUsertype, and the customuseCurrentTimehook is clear.
8-11: Interface definition is suitable.
The interface cleanly declares the props needed by the component.
13-16: Component initialization looks good.
No issues with prop destructuring.
18-22: Time formatting approach is fine.
UsingIntl.DateTimeFormatto retrieve the hour in 24-hour format is a valid approach.
23-27: Short date format is consistent.
The usage of 'month: "short"' and 'day: "numeric"' is good for a concise date display.
28-31: Weekday formatting is appropriate.
No issues.
32-37: User timezone integration is good.
Specifyinguser?.user_timezoneensures correctness across different zones.
39-39: Greeting logic is straightforward and effective.
No issues with the conditional flow.
41-63: JSX structure is clear.
The greeting, date/time display, and optional manage-widgets button are well laid out. Consider i18n if multilingual support is planned.web/core/components/home/widgets/recents/page.tsx (3)
1-9: Imports and type usage look correct.
The usage of types from@plane/types, along with the helper functioncalculateTimeAgo, is clear.
10-14: BlockProps definition is well-formed.
Prop structure for user references and workspace slug is straightforward.
15-23: Component initial setup is correct.
Destructuring props, using router, and retrieving user details withuseMemberis well-structured.web/core/components/home/widgets/manage/widget.helpers.ts (3)
1-9: TargetData type is clearly defined.
No issues, the shape is well suited for the drag-and-drop context.
18-45: Drop instruction logic is coherent.
The logic for adjusting instructions based on group/child relationships is well-structured. Consider verifying corner cases if further complexities arise in drag scenarios.
53-62: Can-drop check is concise.
Returning false if the source is the same as the target prevents erroneous drops.web/core/components/home/widgets/links/links.tsx (5)
1-9: Import organization is clear and minimal.
All necessary modules are imported, and there are no apparent unused imports. Good job keeping imports modular and focused.
10-15: Type definitions are well-defined and easy to understand.
Declaring separate types (TLinkOperationsModalandTProjectLinkList) improves clarity and helps ensure type safety.
17-27: Observer usage appears correct.
Wrapping the functional component inobserverensures MobX state reactivity. Be sure to only read observable data inside the render or in theuseEffecthooks to avoid potential reactivity pitfalls.
60-63: Verify slicing logic.
links.slice(0, 2 * columnCount - 1)might unintentionally exclude or misalign items if the user resizes the window in quick succession. Ensure this logic matches the intended design for link display.
64-75: “Show more” toggling is straightforward.
The mechanism is correct for toggling additional links. This is a clean, minimal approach for progressive disclosure.web/core/components/home/widgets/recents/project.tsx (5)
1-7: Imports are concise and well-structured.
No redundant imports detected. The usage of Next.js router and other dependencies is aligned with the component’s needs.
8-12: Prop definitions are type-safe.
TheBlockPropstype nicely documents the expected props, especially with typedrefandworkspaceSlug.
20-36: Rendering layout is fluid and readable.
Combining the logo, project identifier, name, and timestamp in one row is clear and user-friendly. The chosen Tailwind classes appear consistent with the overall UI style.
64-68: Click handler navigates correctly.
Preventing default and pushing to the project’s issues is a solid approach. Confirm that the workspace slug is always valid to avoid broken links.
13-19: Potential null checks forprojectDetails.
AlthoughprojectDetailsis derived fromactivity.entity_data, it might be worth verifying thatactivity.entity_datais valid to avoid runtime errors.- const projectDetails: TProjectEntityData = activity.entity_data as TProjectEntityData; + const projectDetails: TProjectEntityData | undefined = activity.entity_data as TProjectEntityData; + if (!projectDetails) { + return null; // or some fallback UI + }web/core/components/home/widgets/recents/index.tsx (5)
1-3: Client-side usage with SWR is appropriate.
Using theuse clientdirective and SWR for data fetching is a solid approach for this widget's real-time data needs.
6-8: Props & additional components
ImportingTHomeWidgetProps,FiltersDropdown, and related components keeps this file focused. The layering of filter logic infilters.tsxis a good separation of concerns.
28-34: Widget fosters custom reusability.
RecentActivityWidgetusesTHomeWidgetPropsfor essential data and cleanly manages local states and references. This design can be extended or re-purposed for similar widgets.
51-62: Ensures correct component rendering based onentity_name.
resolveRecent()is a neat switch-based approach. If new entity types are introduced, it’s straightforward to extend.
64-65: Empty state is helpful when no data.
Returning<EmptyWorkspace />for zero results is user-friendly. Good job including specific empty-state UI instead of a blank screen.web/core/components/core/list/list-item.tsx (3)
21-21: New optional propertyitemClassNameimproves flexibility.
Allowing unique styling for each list item container is beneficial for various use cases. Just confirm it doesn’t conflict with parent-level class names.
42-42: Default assignment of empty string is good.
This ensures a fallback and avoids undefined classes.
66-66: Conditional application ofitemClassNameis straightforward.
Usingcnto combine default classes and customitemClassNameis consistent with established patterns.web/core/components/home/widgets/recents/issue.tsx (1)
53-59: Confirm the purpose of the empty onChange callback.Here,
onChangeis a no-op, which might be intentional if you want to display assigned members without editing. If you'd like to handle future changes, consider implementing logic or removing the prop if not needed.web/core/components/home/widgets/links/create-update-link-modal.tsx (1)
60-63: Avoid unmount-time resets that may discard unsaved user input.
reset(defaultValues)on unmount can potentially erase state that might need to persist, such as partially entered data. Evaluate whether this is intended. Otherwise, remove the unmount reset or handle it conditionally.web/core/store/workspace/index.ts (1)
34-34: Integration of thehomestore is well-structured.Adding
home: IHomeStore;to the interface ensures type consistency and self-documentation in the store. This looks good and aids clarity for future maintainers.web/core/layouts/auth-layout/workspace-wrapper.tsx (2)
17-17: Clean use of new hook import.
AddinguseProjectStateimport is consistent with the store usage pattern here. Nothing critical to address.
51-51: Ensure correct initialization offetchWorkspaceStates.
It’s good to see the new function being de-structured here. Consider verifying that the store’s default values (if any) are set properly, so that the rest of the component has consistent state data.web/core/services/workspace.service.ts (1)
15-18: New type imports.
The addition ofTLinkandTWidgetEntityDataclarifies the data structures used in the new methods below. This is a good step toward stronger type safety.web/core/components/home/widgets/loaders/loader.tsx (1)
1-25: NewWidgetLoadercomponent design.
This is a straightforward and clean implementation for conditionally rendering loaders. Good use of an enum for consistency. No immediate issues found.apiserver/plane/app/serializers/workspace.py (1)
164-165: Switched from detailed serializer tovalues_list.
Returning a list of member IDs is likely more efficient, but ensure that any front-end consumers expecting full member data adjust accordingly. The line also exceeds 88 characters; you may want to wrap it for readability.🧰 Tools
🪛 Ruff (0.8.2)
164-164: Line too long (98 > 88)
(E501)
| reorderWidget = async (workspaceSlug: string, widgetKey: string, destinationId: string, edge: string | undefined) => { | ||
| try { | ||
| let resultSequence = 10000; | ||
| if (edge) { | ||
| const sortedIds = orderBy(Object.values(this.widgetsMap), "sort_order", "desc").map((widget) => widget.key); | ||
| const destinationSequence = this.widgetsMap[destinationId]?.sort_order || undefined; | ||
| if (destinationSequence) { | ||
| const destinationIndex = sortedIds.findIndex((id) => id === destinationId); | ||
| if (edge === "reorder-above") { | ||
| const prevSequence = this.widgetsMap[sortedIds[destinationIndex - 1]]?.sort_order || undefined; | ||
| if (prevSequence) { | ||
| resultSequence = (destinationSequence + prevSequence) / 2; | ||
| } else { | ||
| resultSequence = destinationSequence + resultSequence; | ||
| } | ||
| } else { | ||
| resultSequence = destinationSequence - resultSequence; | ||
| } | ||
| } | ||
| } | ||
| await this.workspaceService.updateWorkspaceWidget(workspaceSlug, widgetKey, { | ||
| sort_order: resultSequence, | ||
| }); | ||
| runInAction(() => { | ||
| set(this.widgetsMap, [widgetKey, "sort_order"], resultSequence); | ||
| }); | ||
| } catch (error) { | ||
| console.error("Failed to move widget"); | ||
| throw error; | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check boundary conditions for reorder logic.
The calculation of new sort_order uses a hard-coded 10000 offset, which might produce collisions if repeated multiple times. For instance, multiple reorder operations with minimal intervals might eventually cause duplicates or require rebalancing. Consider a dynamic rebalancing or a server-managed approach for a more scalable ordering.
| export const useHome = (): IHomeStore => { | ||
| const context = useContext(StoreContext); | ||
| if (context === undefined) throw new Error("useDashboard must be used within StoreProvider"); | ||
| return context.workspaceRoot.home; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error messages and naming clarity.
The error references useDashboard while the hook is named useHome. Update or clarify the error message to ensure consistent naming. Also consider giving context on how to provide the StoreProvider.
- 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.
| export const useHome = (): IHomeStore => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useDashboard must be used within StoreProvider"); | |
| return context.workspaceRoot.home; | |
| }; | |
| export const useHome = (): IHomeStore => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useHome must be used within StoreProvider"); | |
| return context.workspaceRoot.home; | |
| }; |
| import { TLogoProps } from "./common"; | ||
| import { TIssuePriorities } from "./issues"; | ||
|
|
||
| export type TRecentActivityFilterKeys = "all item" | "issue" | "page" | "project"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Revisit the "all item" literal.
The union type "all item" is potentially ambiguous or unintentional. Consider renaming to "all" or "all_items" to improve clarity and avoid confusion when filtering recent activities.
| export interface IWorkspaceLinkStoreActions { | ||
| addLinks: (projectId: string, links: TLink[]) => void; | ||
| fetchLinks: (workspaceSlug: string) => Promise<TLink[]>; | ||
| createLink: (workspaceSlug: string, data: Partial<TLink>) => Promise<TLink>; | ||
| updateLink: (workspaceSlug: string, linkId: string, data: Partial<TLink>) => Promise<TLink>; | ||
| removeLink: (workspaceSlug: string, linkId: string) => Promise<void>; | ||
| setLinkData: (link: TLink | undefined) => void; | ||
| toggleLinkModal: (isOpen: boolean) => void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameter naming inconsistency in the addLinks interface vs. implementation.
In the interface, the method signature suggests addLinks receives (projectId: string, links: TLink[]), but the implementation uses (workspaceSlug: string, links: TLink[]). This may introduce confusion or incorrect usage. Consider aligning the parameter names for clarity.
-export interface IWorkspaceLinkStoreActions {
- addLinks: (projectId: string, links: TLink[]) => void;
+export interface IWorkspaceLinkStoreActions {
+ addLinks: (workspaceSlug: string, links: TLink[]) => void;
...
}📝 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.
| export interface IWorkspaceLinkStoreActions { | |
| addLinks: (projectId: string, links: TLink[]) => void; | |
| fetchLinks: (workspaceSlug: string) => Promise<TLink[]>; | |
| createLink: (workspaceSlug: string, data: Partial<TLink>) => Promise<TLink>; | |
| updateLink: (workspaceSlug: string, linkId: string, data: Partial<TLink>) => Promise<TLink>; | |
| removeLink: (workspaceSlug: string, linkId: string) => Promise<void>; | |
| setLinkData: (link: TLink | undefined) => void; | |
| toggleLinkModal: (isOpen: boolean) => void; | |
| } | |
| export interface IWorkspaceLinkStoreActions { | |
| addLinks: (workspaceSlug: string, links: TLink[]) => void; | |
| fetchLinks: (workspaceSlug: string) => Promise<TLink[]>; | |
| createLink: (workspaceSlug: string, data: Partial<TLink>) => Promise<TLink>; | |
| updateLink: (workspaceSlug: string, linkId: string, data: Partial<TLink>) => Promise<TLink>; | |
| removeLink: (workspaceSlug: string, linkId: string) => Promise<void>; | |
| setLinkData: (link: TLink | undefined) => void; | |
| toggleLinkModal: (isOpen: boolean) => void; | |
| } |
pushya22
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apiserver/plane/app/serializers/workspace.py (1)
153-153: Format the fields list for better readability.The addition of the "id" field is good. However, let's improve the formatting to meet PEP 8 guidelines.
- fields = ["id", "name", "state", "priority", "assignees", "type", "sequence_id", "project_id", "project_identifier", ] + fields = [ + "id", + "name", + "state", + "priority", + "assignees", + "type", + "sequence_id", + "project_id", + "project_identifier", + ]🧰 Tools
🪛 Ruff (0.8.2)
153-153: Line too long (134 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/app/serializers/workspace.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/serializers/workspace.py
153-153: Line too long (134 > 88)
(E501)
169-169: Line too long (98 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: lint-apiserver
- GitHub Check: lint-web
- GitHub Check: lint-space
- GitHub Check: lint-admin
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apiserver/plane/app/serializers/workspace.py (1)
169-171: Verify the impact of simplified project members response.The optimization to return only member IDs instead of full member details is good for performance. However, this is a breaking change that might affect API consumers.
Let's verify the impact:
Also, consider improving the formatting:
- members = ProjectMember.objects.filter(project_id=obj.id).values_list("member", flat=True) + members = ( + ProjectMember.objects.filter(project_id=obj.id) + .values_list("member", flat=True) + )✅ Verification successful
Breaking change verification completed - safe to proceed
The simplified project_members response is safe to implement as this serializer is only used internally for entity mapping and is not directly exposed through any API endpoints. No API consumers will be affected by this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for API consumers that might be affected by the breaking change # Search for references to project_members in the frontend code echo "Searching for project_members usage in frontend code..." rg -A 3 "project_members" --type ts --type tsx # Search for references to ProjectRecentVisitSerializer echo "Searching for direct API calls..." rg -A 3 "ProjectRecentVisitSerializer|/api/projects.*members" --type pyLength of output: 969
Script:
#!/bin/bash # Search frontend with correct file types echo "Searching frontend code..." rg -A 3 "project_members" --type js --type jsx # Search for views/viewsets using the serializer echo "Searching for views using ProjectRecentVisitSerializer..." rg -A 5 "ProjectRecentVisitSerializer" --type py # Search for API routes related to recent visits echo "Searching for API routes..." rg -A 3 "recent.*visit|visit.*recent" --type pyLength of output: 11836
🧰 Tools
🪛 Ruff (0.8.2)
169-169: Line too long (98 > 88)
(E501)
Summary
This PR implements home screen revamp.
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the updated release notes:
New Features
Improvements
Bug Fixes
Performance