[WEB-5785]fix: favorites icon size #8449
Conversation
📝 WalkthroughWalkthroughTwo sidebar component files updated with UI refinements and control-flow improvements: replaced button with IconButton for folder creation, added early returns after success toasts in promise handlers, and adjusted icon sizing using Tailwind utility classes. Imports were cleaned up and empty comment blocks removed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx (2)
68-86: State reset executes before async operation completes.Lines 84-85 execute synchronously immediately after setting up the promise chain, not after the API call completes. This means the form is reset (
setCreateNewFolder(false),setValue("name", "")) beforeaddFavoritefinishes, which could lead to:
- Form closing while the API request is still in flight
- User unable to see loading state
- Potential issues if user interacts with UI before completion
These state updates should be moved inside the
.then()block to execute only after successful completion.🔎 Proposed fix
addFavorite(workspaceSlug.toString(), formData) .then(() => { setToast({ type: TOAST_TYPE.SUCCESS, title: t("success"), message: t("favorite_created_successfully"), }); - return; + setCreateNewFolder(false); + setValue("name", ""); }) .catch(() => { setToast({ type: TOAST_TYPE.ERROR, title: t("error"), message: t("something_went_wrong"), }); }); -setCreateNewFolder(false); -setValue("name", "");
107-125: Same issue: state reset executes before async operation completes.Lines 123-124 reset the form state synchronously, before
updateFavoritecompletes. Move these inside the.then()block for proper async flow.🔎 Proposed fix
updateFavorite(workspaceSlug.toString(), favoriteId, payload) .then(() => { setToast({ type: TOAST_TYPE.SUCCESS, title: t("success"), message: t("favorite_updated_successfully"), }); - return; + setCreateNewFolder(false); + setValue("name", ""); }) .catch(() => { setToast({ type: TOAST_TYPE.ERROR, title: t("error"), message: t("something_went_wrong"), }); }); -setCreateNewFolder(false); -setValue("name", "");
🧹 Nitpick comments (1)
apps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (1)
113-113: Optional: explicit return has no behavioral effect.The
return;statement after the success toast doesn't change behavior since there's no code following it in the.then()block. While it may clarify intent to exit early, it's functionally redundant.Consider removing if preferred
deleteFavorite(workspaceSlug.toString(), favorite.id) .then(() => { setToast({ type: TOAST_TYPE.SUCCESS, title: t("success"), message: t("favorite_removed_successfully"), }); - return; })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsxapps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsxapps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsxapps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsxapps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsxapps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Sidebar menu items should use transparent backgrounds with hover states like `hover:bg-layer-1-hover` rather than base layer backgrounds, maintaining visual lightness while providing hover feedback
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Sidebar menu items should use transparent backgrounds with hover states like `hover:bg-layer-1-hover` rather than base layer backgrounds, maintaining visual lightness while providing hover feedback
Applied to files:
apps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build packages
🔇 Additional comments (3)
apps/web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx (1)
136-136: LGTM! Icon sizing fix addresses the PR objective.Changing from
w-[16px](width-only) tosize-4(width + height) ensures the icon renders as a proper 16×16px square, which fixes the inconsistent icon size issue mentioned in the PR.apps/web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (2)
1-1: LGTM! Correct removal of unused React import.With the modern JSX transform and globally available React types, the default React import is unnecessary.
204-213: LGTM! IconButton provides better semantics for icon-only action.The refactor from
ButtontoIconButtonwith appropriatevariant="ghost"andsize="sm"props improves UI consistency and semantics for the folder creation action.
Description
This update fixes the inconsistent icon size for favorties header.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.