-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[WIKI-874] refactor: description input component #8544
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?
Conversation
|
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. 📝 WalkthroughWalkthroughDescriptionInput's public API changes from a string parameter to an object containing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,mts,cts}📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx,json,css}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-10T13:25:14.810ZApplied to files:
🧬 Code graph analysis (1)apps/web/core/components/editor/rich-text/description-input/root.tsx (3)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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.
Pull request overview
This PR refactors the DescriptionInput component to pass both description_html and description_json in the onSubmit callback, preparing the codebase for future use of JSON descriptions alongside HTML descriptions.
Changes:
- Modified the
DescriptionInputcomponent'sonSubmitsignature to accept an object containing bothdescription_htmlanddescription_json - Updated the component to capture and pass
description_jsonfrom the editor'sonChangecallback - Updated all three consumer components to extract
description_htmlfrom the new object structure
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/web/core/components/editor/rich-text/description-input/root.tsx | Refactored component to capture and pass description_json along with description_html in the onSubmit callback |
| apps/web/core/components/issues/peek-overview/issue-detail.tsx | Updated to extract description_html from the new value object structure |
| apps/web/core/components/issues/issue-detail/main-content.tsx | Updated to extract description_html from the new value object structure |
| apps/web/core/components/inbox/content/issue-root.tsx | Updated to extract description_html from the new value object structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
154-168: Reset effect does not cleardescription_json.When the form resets (e.g., on
entityIdorinitialValuechange),description_jsonis not reset in either the form state orlocalDescription. This could cause stale JSON data to persist across entity switches.Suggested fix
useEffect(() => { if (!entityId) return; reset({ id: entityId, description_html: initialValue?.trim() === "" ? "<p></p>" : (initialValue ?? "<p></p>"), + description_json: undefined, isMigrationUpdate: false, }); setLocalDescription({ id: entityId, description_html: initialValue?.trim() === "" ? "<p></p>" : (initialValue ?? "<p></p>"), + description_json: undefined, isMigrationUpdate: false, }); // Reset unsaved changes flag when form is reset hasUnsavedChanges.current = false; }, [entityId, initialValue, reset]);
🤖 Fix all issues with AI agents
In `@apps/web/core/components/editor/rich-text/description-input/root.tsx`:
- Line 107: The prop issueSequenceId is destructured but not forwarded to the
RichTextEditor instance; update the RichTextEditor invocation in this component
to include issueSequenceId (e.g., pass issueSequenceId={issueSequenceId}) so the
editor receives the prop accepted by the RichTextEditor in editor.tsx.
🧹 Nitpick comments (2)
apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
116-120:description_jsonnot initialized in local state.The
localDescriptionstate is missing thedescription_jsonfield, which could lead to stale or undefined values if accessed before the firstonChangeevent. Consider initializing it for consistency withTFormData.Suggested fix
const [localDescription, setLocalDescription] = useState<TFormData>({ id: entityId, description_html: initialValue?.trim() ?? "", + description_json: undefined, isMigrationUpdate: false, });apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
87-92: Pre-existing bug: Condition is always true.The condition on line 88 uses
||(OR) instead of&&(AND), making it always evaluate totrue. Any value satisfies "not undefined OR not null". This is pre-existing code but worth noting for a future fix.Suggested fix
const issueDescription = - issue.description_html !== undefined || issue.description_html !== null + issue.description_html !== undefined && issue.description_html !== null ? issue.description_html != "" ? issue.description_html : "<p></p>" : undefined;Or simplify with nullish checks:
-const issueDescription = - issue.description_html !== undefined || issue.description_html !== null - ? issue.description_html != "" - ? issue.description_html - : "<p></p>" - : undefined; +const issueDescription = + issue.description_html != null + ? (issue.description_html !== "" ? issue.description_html : "<p></p>") + : undefined;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/core/components/editor/rich-text/description-input/root.tsxapps/web/core/components/inbox/content/issue-root.tsxapps/web/core/components/issues/issue-detail/main-content.tsxapps/web/core/components/issues/peek-overview/issue-detail.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/issues/issue-detail/main-content.tsxapps/web/core/components/issues/peek-overview/issue-detail.tsxapps/web/core/components/inbox/content/issue-root.tsxapps/web/core/components/editor/rich-text/description-input/root.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-detail/main-content.tsxapps/web/core/components/issues/peek-overview/issue-detail.tsxapps/web/core/components/inbox/content/issue-root.tsxapps/web/core/components/editor/rich-text/description-input/root.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-detail/main-content.tsxapps/web/core/components/issues/peek-overview/issue-detail.tsxapps/web/core/components/inbox/content/issue-root.tsxapps/web/core/components/editor/rich-text/description-input/root.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-detail/main-content.tsxapps/web/core/components/issues/peek-overview/issue-detail.tsxapps/web/core/components/inbox/content/issue-root.tsxapps/web/core/components/editor/rich-text/description-input/root.tsx
🧠 Learnings (1)
📚 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/inbox/content/issue-root.tsxapps/web/core/components/editor/rich-text/description-input/root.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). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build packages
🔇 Additional comments (5)
apps/web/core/components/editor/rich-text/description-input/root.tsx (2)
140-151: LGTM!The submit handler correctly restructures the payload to pass both
description_htmlanddescription_jsonto theonSubmitcallback, aligning with the new API contract.
226-232: LGTM!The
onChangehandler correctly capturesdescription_jsonfrom the editor and stores it in the form state viasetValue.apps/web/core/components/issues/issue-detail/main-content.tsx (1)
137-143: LGTM!The
onSubmithandler correctly extractsdescription_htmlfrom the new object-basedvalueparameter, aligning with the refactoredDescriptionInputAPI.apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
137-143: LGTM!The
onSubmithandler correctly extractsdescription_htmlfrom the new object-basedvalueparameter, consistent with the refactoredDescriptionInputAPI and other consumers in this PR.apps/web/core/components/inbox/content/issue-root.tsx (1)
173-179: LGTM!The
onSubmithandler correctly extractsdescription_htmlfrom the new object-basedvalueparameter, consistent with the refactoredDescriptionInputAPI and other consumers in this PR.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Description
This PR refactors the description input component to pass
description_jsonas a part of theonSubmitcallback function.Type of Change
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.