-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-1201] chore: dropdown options hierarchy improvements #8501
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
base: preview
Are you sure you want to change the base?
[WEB-1201] chore: dropdown options hierarchy improvements #8501
Conversation
📝 WalkthroughWalkthroughThis pull request adds sorting utility functions for dropdown options and propagates a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this 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 (2)
packages/utils/src/array.ts (2)
219-228: PrefertoSorted()for immutability.The current implementation uses
[...options].sort()to avoid mutation. Per the project's TypeScript guidelines, prefer thetoSorted()method which is more explicit about immutability and doesn't require manual spreading.♻️ Refactor to use toSorted()
- // Create a shallow copy to avoid mutating the original array - return [...options].sort((a, b) => { + return options.toSorted((a, b) => { const aSelected = a.value !== null && selectedSet.has(a.value); const bSelected = b.value !== null && selectedSet.has(b.value); // If both selected or both unselected, maintain original order if (aSelected === bSelected) return 0; // Selected items come first return aSelected ? -1 : 1; });As per coding guidelines, TypeScript 5.2+ supports copying array methods for immutable operations.
253-271: PrefertoSorted()for immutability.Similar to
sortBySelectedFirst, this function uses[...options].sort()which works but isn't the preferred pattern per project guidelines. UsetoSorted()for more explicit immutability.♻️ Refactor to use toSorted()
- // Create a shallow copy to avoid mutating the original array - return [...options].sort((a, b) => { + return options.toSorted((a, b) => { const aIsCurrent = currentUserId && a.value === currentUserId; const bIsCurrent = currentUserId && b.value === currentUserId; // Current user always comes first if (aIsCurrent && !bIsCurrent) return -1; if (!aIsCurrent && bIsCurrent) return 1; if (aIsCurrent && bIsCurrent) return 0; // If neither is current user, sort by selection state const aSelected = a.value !== null && selectedSet.has(a.value); const bSelected = b.value !== null && selectedSet.has(b.value); // If both selected or both unselected, maintain original order if (aSelected === bSelected) return 0; // Selected items come before unselected return aSelected ? -1 : 1; });As per coding guidelines, TypeScript 5.2+ supports copying array methods for immutable operations.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/core/components/dropdowns/member/base.tsxapps/web/core/components/dropdowns/member/member-options.tsxapps/web/core/components/dropdowns/module/base.tsxapps/web/core/components/dropdowns/module/module-options.tsxapps/web/core/components/dropdowns/project/base.tsxapps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsxpackages/utils/src/array.ts
🧰 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/issues/issue-layouts/properties/label-dropdown.tsxapps/web/core/components/dropdowns/member/member-options.tsxapps/web/core/components/dropdowns/module/module-options.tsxapps/web/core/components/dropdowns/member/base.tsxpackages/utils/src/array.tsapps/web/core/components/dropdowns/project/base.tsxapps/web/core/components/dropdowns/module/base.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsxapps/web/core/components/dropdowns/member/member-options.tsxapps/web/core/components/dropdowns/module/module-options.tsxapps/web/core/components/dropdowns/member/base.tsxpackages/utils/src/array.tsapps/web/core/components/dropdowns/project/base.tsxapps/web/core/components/dropdowns/module/base.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/issues/issue-layouts/properties/label-dropdown.tsxapps/web/core/components/dropdowns/member/member-options.tsxapps/web/core/components/dropdowns/module/module-options.tsxapps/web/core/components/dropdowns/member/base.tsxpackages/utils/src/array.tsapps/web/core/components/dropdowns/project/base.tsxapps/web/core/components/dropdowns/module/base.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/issues/issue-layouts/properties/label-dropdown.tsxapps/web/core/components/dropdowns/member/member-options.tsxapps/web/core/components/dropdowns/module/module-options.tsxapps/web/core/components/dropdowns/member/base.tsxpackages/utils/src/array.tsapps/web/core/components/dropdowns/project/base.tsxapps/web/core/components/dropdowns/module/base.tsx
🧠 Learnings (6)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use copying array methods (`toSorted`, `toSpliced`, `with`) for immutable array operations (TypeScript 5.2+)
Applied to files:
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsxpackages/utils/src/array.tsapps/web/core/components/dropdowns/project/base.tsx
📚 Learning: 2025-10-10T13:25:14.810Z
Learnt from: gakshita
Repo: makeplane/plane PR: 7949
File: apps/web/core/components/issues/issue-modal/form.tsx:183-189
Timestamp: 2025-10-10T13:25:14.810Z
Learning: In `apps/web/core/components/issues/issue-modal/form.tsx`, the form reset effect uses a `dataResetProperties` dependency array prop (default: []) to give parent components explicit control over when the form resets. Do not suggest adding the `data` prop itself to the dependency array, as this would cause unwanted resets on every render when the data object reference changes, disrupting user input. The current pattern is intentional and allows the parent to trigger resets only when specific conditions are met.
Applied to files:
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsxapps/web/core/components/dropdowns/member/member-options.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/issues/issue-layouts/properties/label-dropdown.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/plane/ui/**/*.{ts,tsx} : Build components in `plane/ui` package with Storybook for isolated development
Applied to files:
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsx
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
Repo: makeplane/plane PR: 6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
Applied to files:
apps/web/core/components/dropdowns/module/module-options.tsx
🧬 Code graph analysis (4)
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsx (1)
packages/utils/src/array.ts (1)
sortBySelectedFirst(207-229)
apps/web/core/components/dropdowns/member/member-options.tsx (1)
packages/utils/src/array.ts (1)
sortByCurrentUserThenSelected(242-272)
apps/web/core/components/dropdowns/module/module-options.tsx (1)
packages/utils/src/array.ts (1)
sortBySelectedFirst(207-229)
apps/web/core/components/dropdowns/project/base.tsx (1)
packages/utils/src/array.ts (1)
sortBySelectedFirst(207-229)
⏰ 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). (3)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build packages
🔇 Additional comments (13)
apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsx (3)
17-17: LGTM! Correct import for dropdown hierarchy improvement.The import of
sortBySelectedFirstaligns with the PR objective to improve dropdown options hierarchy by prioritizing selected items.
114-121: LGTM! Correct implementation of selected-first sorting.The
filteredOptionscomputation correctly wraps the existing filter logic withsortBySelectedFirstto prioritize selected labels. The dependency array properly includesvalueto ensure re-computation when selections change.
271-271: LGTM! Appropriate null-safety improvement.The added null check is necessary since
sortBySelectedFirstcan returnundefined. This prevents potential runtime errors when accessing thelengthproperty.apps/web/core/components/dropdowns/module/base.tsx (1)
190-190: LGTM! Correct prop propagation for value-aware sorting.Forwarding the
valueprop toModuleOptionsenables the component to implement selected-first sorting, consistent with the PR's objective to improve dropdown hierarchy.apps/web/core/components/dropdowns/member/base.tsx (1)
180-180: LGTM! Correct prop propagation for value-aware sorting.Forwarding the
valueprop toMemberOptionsenables the component to implement current-user-then-selected sorting, aligning with the PR's dropdown hierarchy improvements.apps/web/core/components/dropdowns/project/base.tsx (2)
11-11: LGTM! Correct import for dropdown hierarchy improvement.The import of
sortBySelectedFirstenables prioritizing selected projects in the dropdown options.
113-119: LGTM! Excellent use of type predicates and sorting.The filtering logic correctly:
- Excludes the current project
- Filters by search query when present
- Uses a type predicate
(o): o is NonNullable<typeof o>to properly narrow the type from(T | undefined)[]toT[]- Sorts selected projects first
The type predicate is excellent TypeScript practice that provides compile-time type safety.
apps/web/core/components/dropdowns/member/member-options.tsx (3)
14-14: LGTM! Correct import for member hierarchy sorting.The import of
sortByCurrentUserThenSelectedenables the dropdown to prioritize the current user first, then selected members, improving UX.
29-29: LGTM! Correct prop type for value-aware sorting.The
valueprop is correctly typed as optional with a union type (string[] | string | null) to support both single and multiple selection modes.
116-120: LGTM! Correct implementation of hierarchical member sorting.The
filteredOptionscomputation correctly usessortByCurrentUserThenSelectedto establish a clear hierarchy:
- Current user appears first
- Selected members appear next
- Other members appear last
This significantly improves UX by making it easy to find relevant members.
apps/web/core/components/dropdowns/module/module-options.tsx (3)
10-10: LGTM!The import correctly references the new utility function added to the shared utils package.
30-30: LGTM!The
valueprop is correctly typed to support both single-select (string | null) and multi-select (string[]) scenarios, matching the signature expected bysortBySelectedFirst.
104-107: LGTM!The filtering and sorting logic is well-structured: it first applies text-based filtering when a query exists, then uses
sortBySelectedFirstto prioritize selected items. Thevalueprop is correctly threaded through to enable this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves dropdown options hierarchy by implementing sorting functions that prioritize selected items and current users in dropdown lists. The changes enhance user experience by making relevant options more accessible.
Key Changes:
- Added two new utility functions (
sortBySelectedFirstandsortByCurrentUserThenSelected) for sorting dropdown options - Integrated sorting logic across label, project, module, and member dropdowns
- Enhanced null/undefined handling in the project dropdown
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/array.ts | Added two new sorting utility functions with comprehensive JSDoc documentation for sorting dropdown options by selection state and current user |
| apps/web/core/components/issues/issue-layouts/properties/label-dropdown.tsx | Integrated sortBySelectedFirst to prioritize selected labels in the dropdown |
| apps/web/core/components/dropdowns/project/base.tsx | Applied sortBySelectedFirst with improved undefined filtering for project options |
| apps/web/core/components/dropdowns/module/module-options.tsx | Added value prop and integrated sortBySelectedFirst for module sorting |
| apps/web/core/components/dropdowns/module/base.tsx | Passed value prop to ModuleOptions component for sorting support |
| apps/web/core/components/dropdowns/member/member-options.tsx | Added value prop and integrated sortByCurrentUserThenSelected to prioritize current user and selected members |
| apps/web/core/components/dropdowns/member/base.tsx | Passed value prop to MemberOptions component for sorting support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This PR includes improvement for dropdown options hierarchy.
Type of Change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.