[WIKI-774] refactor: space app mentions #8152
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR removes public re-exports from the embeds module hierarchy, introduces a type alias for mention component props, updates imports accordingly, and refactors the state property component to accept either state details or state ID with runtime resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Consumer Component
participant StateComponent as IssueBlockState
participant Store as State Store
Consumer->>StateComponent: Pass props (stateId or stateDetails)
alt Has stateId
StateComponent->>Store: getStateById(stateId)
Store-->>StateComponent: Return resolved state
else Has stateDetails
StateComponent->>StateComponent: Use provided stateDetails
end
StateComponent->>StateComponent: Guard check (render nothing if falsy)
StateComponent-->>Consumer: Render with state.name and state.group
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 20
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| <div className="flex w-full items-center gap-1.5"> | ||
| <StateGroupIcon stateGroup={state?.group ?? "backlog"} color={state?.color} /> | ||
| <div className="text-xs">{state?.name ?? "State"}</div> | ||
| <StateGroupIcon stateGroup={state.group} /> |
There was a problem hiding this comment.
Bug: StateGroupIcon missing color prop in refactored component
The StateGroupIcon component is called without the color prop. When state is retrieved via getStateById(), it contains a color property from the IState object that should be passed to the component. The original code passed color={state?.color}, but the refactored code removed this, causing custom state colors to be ignored in favor of default group colors.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the space app mentions implementation to improve scalability by consolidating type definitions and clarifying import paths. The refactoring introduces a wrapper type TEditorMentionComponentProps in the CE layer and updates imports to use more explicit paths, while also enhancing the IssueBlockState component to support both state ID lookups and direct state details.
Key Changes:
- Updated
IssueBlockStateto accept eitherstateIdorstateDetailsvia discriminated union type - Consolidated mention component types by creating
TEditorMentionComponentPropsas an alias forTCallbackMentionComponentProps - Updated import paths to be more explicit (
@/plane-web/components/editor/embeds/mentionsinstead of shorter paths)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/space/core/components/issues/issue-layouts/properties/state.tsx | Refactored to support both stateId and stateDetails props, improved null safety, simplified StateGroupIcon usage |
| apps/space/core/components/editor/embeds/mentions/root.tsx | Updated imports to use explicit paths and new type alias TEditorMentionComponentProps |
| apps/space/ce/components/editor/index.ts | Deleted file - creates breaking change for ee/components/editor re-exports |
| apps/space/ce/components/editor/embeds/mentions/root.tsx | Added type export TEditorMentionComponentProps as wrapper for TCallbackMentionComponentProps |
| apps/space/ce/components/editor/embeds/index.ts | Deleted file - part of import path restructuring |
Comments suppressed due to low confidence (1)
apps/space/ce/components/editor/index.ts:1
- Deleting this file breaks the re-export in
apps/space/ee/components/editor/index.ts, which hasexport * from "ce/components/editor";. This will cause a module resolution error. Consider either:
- Keeping this file and updating it to re-export from the correct path, or
- Updating
apps/space/ee/components/editor/index.tsto export from the correct path directly
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🧹 Nitpick comments (1)
apps/space/core/components/issues/issue-layouts/properties/state.tsx (1)
13-45: Props union + runtime resolution look good; clarify StoreProvider expectationsThe new
Propsunion (stateDetailsvsstateId) plus the derivedstateand guard:const state = "stateId" in props ? getStateById(props.stateId) : props.stateDetails; if (!state) return null;give you a clean, type‑safe way to either resolve from the store or pass in pre‑resolved details, and the early return nicely removes the need for optional chaining/fallbacks in the JSX. The tooltip/icon usage of
state.name/state.groupis straightforward.One thing to be aware of:
useStates()is still called unconditionally at the top of the component, soIssueBlockStatestill requires being rendered underStoreProvidereven when onlystateDetailsare passed. If the intent was purely ergonomics (callers can avoidgetStateByIdthemselves) that’s fine; if the goal was to enable a store‑independent “pure props” usage (e.g., in places without the store context), this will still throw viauseStatesand might warrant either a separate pure component or hoisting the store access up to callers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/space/ce/components/editor/embeds/index.ts(0 hunks)apps/space/ce/components/editor/embeds/mentions/root.tsx(1 hunks)apps/space/ce/components/editor/index.ts(0 hunks)apps/space/core/components/editor/embeds/mentions/root.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/properties/state.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/space/ce/components/editor/embeds/index.ts
- apps/space/ce/components/editor/index.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
📚 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/space/ce/components/editor/embeds/mentions/root.tsxapps/space/core/components/editor/embeds/mentions/root.tsx
🧬 Code graph analysis (3)
apps/space/ce/components/editor/embeds/mentions/root.tsx (2)
apps/web/ce/components/editor/embeds/mentions/root.tsx (2)
TEditorMentionComponentProps(4-4)EditorAdditionalMentionsRoot(6-8)packages/editor/src/core/types/mention.ts (1)
TCallbackMentionComponentProps(19-19)
apps/space/core/components/editor/embeds/mentions/root.tsx (3)
apps/web/core/components/editor/embeds/mentions/root.tsx (1)
EditorMentionsRoot(7-16)apps/space/ce/components/editor/embeds/mentions/root.tsx (1)
TEditorMentionComponentProps(4-4)apps/web/ce/components/editor/embeds/mentions/root.tsx (1)
TEditorMentionComponentProps(4-4)
apps/space/core/components/issues/issue-layouts/properties/state.tsx (1)
apps/space/core/hooks/store/use-state.ts (1)
useStates(7-11)
⏰ 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). (4)
- GitHub Check: Build and lint web apps
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/space/core/components/editor/embeds/mentions/root.tsx (1)
1-15: Mentions root now aligned with plane‑web mention types — LGTMUsing
TEditorMentionComponentPropsfrom the sharedplane-webembeds module and wiringEditorAdditionalMentionsRootthrough that same entrypoint keeps the mentions surface consistent across apps while preserving the existing switch logic. Also good call keeping the type‑only import separate from the value import to respect bundler expectations. Based on learningsapps/space/ce/components/editor/embeds/mentions/root.tsx (1)
4-7: Type alias + signature change are consistent with web CE — LGTMAliasing
TEditorMentionComponentPropstoTCallbackMentionComponentPropsand updatingEditorAdditionalMentionsRootto use the alias keeps the type surface coherent with web while preserving behavior (still a no‑op placeholder). This should make cross‑app imports simpler without affecting runtime.
…actor/space-mentions
Description
This PR refactors space app mentions implementation for scalability.
Type of Change
Note
Unifies editor mentions typing/entrypoints and refactors IssueBlockState to accept either state details or ID with minor UI cleanup.
TEditorMentionComponentPropstype alias and use it across mentions components.EditorMentionsRootto importEditorAdditionalMentionsRootand types fromplane-webmentions module.ce/components/editor/{embeds,index}to tighten module surface.IssueBlockStatenow accepts eitherstateIdor inlinestateDetails { name, group }; resolves state conditionally.Written by Cursor Bugbot for commit 76fa1ec. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
Refactor
API Changes
✏️ Tip: You can customize this high-level summary in your review settings.