[WIKI-538] chore: common description component #7785
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. WalkthroughAdds a new DescriptionInput component with autosave, debounced submit, and SWR sync plus a DescriptionInputLoader and an index re-export; replaces multiple IssueDescriptionInput usages with DescriptionInput and moves persist logic to external onSubmit callbacks across inbox and issue views. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant View as Issue View (wrapper)
participant DI as DescriptionInput
participant Ops as issueOperations
participant API as Backend
User->>DI: Edit description (typing)
DI->>DI: Debounced autosave / mark unsaved
DI->>View: onSubmit(value) [debounced or explicit]
View->>Ops: update(workspaceSlug, project_id, id, { description_html: value })
Ops->>API: HTTP PUT /issues/:id
API-->>Ops: 200 OK
Ops-->>View: Promise resolved
View-->>DI: setIsSubmitting(false) / reflect saved state
Note over DI: If initial/swr description missing -> show DescriptionInputLoader
Note right of DI: On unmount -> ensure final save if unsaved changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Pull Request Overview
This PR refactors the issue description input component by replacing multiple feature-specific description components with a generic DescriptionInput component. The refactoring standardizes the API for description inputs across the application while maintaining the same functionality.
- Replaces
IssueDescriptionInputwith the new genericDescriptionInputcomponent - Introduces a reusable description input loader component
- Updates import statements to include new file asset types and utilities
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
apps/web/core/components/issues/peek-overview/issue-detail.tsx |
Updates to use the new DescriptionInput component with standardized props |
apps/web/core/components/issues/issue-detail/main-content.tsx |
Replaces issue-specific description input with generic component |
apps/web/core/components/inbox/content/issue-root.tsx |
Migrates inbox issue description to use the new component and loader |
apps/web/core/components/editor/rich-text/description-input/root.tsx |
Implements the new generic description input component with comprehensive props |
apps/web/core/components/editor/rich-text/description-input/loader.tsx |
Creates a dedicated loader component for description input |
apps/web/core/components/editor/rich-text/description-input/index.ts |
Adds barrel export for the description input components |
Comments suppressed due to low confidence (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx:1
- The
swrDescriptionparameter is nullable butprojectIdis not validated for null/undefined before being passed to the API call. Consider adding validation or making the parameter handling consistent.
"use client";
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/web/core/components/editor/rich-text/description-input/root.tsx
Outdated
Show resolved
Hide resolved
apps/web/core/components/editor/rich-text/description-input/root.tsx
Outdated
Show resolved
Hide resolved
![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
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/issues/peek-overview/issue-detail.tsx (2)
1-1: Fix client directive.Must be
"use client"(space), otherwise the file won’t be treated as a client component.-"use-client"; +"use client";
85-90: Null/undefined check is incorrect (always truthy).Use a nullish check and handle empty string explicitly.
- 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 + ? undefined + : issue.description_html.trim() === "" + ? "<p></p>" + : issue.description_html;
🧹 Nitpick comments (8)
apps/web/core/components/editor/rich-text/description-input/index.ts (1)
1-1: Unify public surface: also re-export the loader from the index.This keeps consumers from importing a sibling path for the loader.
export * from "./root"; +export * from "./loader";apps/web/core/components/editor/rich-text/description-input/loader.tsx (1)
15-17: Tailwind padding classes are redundant.
p-3 py-2 pt-3resolves to top: 0.75rem, bottom: 0.5rem, x: 0.75rem. Use explicit shorthands to avoid overrides.- "min-h-[120px] max-h-64 space-y-2 overflow-hidden rounded-md border border-custom-border-200 p-3 py-2 pt-3", + "min-h-[120px] max-h-64 space-y-2 overflow-hidden rounded-md border border-custom-border-200 px-3 pt-3 pb-2",apps/web/core/components/inbox/content/issue-root.tsx (1)
192-196: Match loader layout with editor container to avoid layout shift.Use the same container classes as the editor for a seamless skeleton.
- {loader === "issue-loading" ? ( - <DescriptionInputLoader /> + {loader === "issue-loading" ? ( + <DescriptionInputLoader className="-ml-3 border-none" />apps/web/core/components/editor/rich-text/description-input/root.tsx (5)
189-207: Bind Controller value to the editor when SWR value is absent to avoid saving stale form data.Controller currently ignores
value, so form state can drift from displayed content if no SWR value is provided. Use form value as a fallback.-render={({ field: { onChange } }) => ( +render={({ field: { onChange, value } }) => ( <RichTextEditor @@ - value={swrDescription ?? null} + value={swrDescription ?? value ?? null} @@ onChange={(_description, description_html) => { setIsSubmitting("submitting"); onChange(description_html); hasUnsavedChanges.current = true; debouncedFormSave(); }}
198-199: Avoid passing empty string as workspaceId.Prefer
undefinedwhen unknown to prevent accidental "truthy" empty ID handling downstream.-workspaceId={workspaceDetails?.id ?? ""} +workspaceId={workspaceDetails?.id}
209-213: Don’t sendproject_id: undefinedin search payload.Drop the key when
projectIdis falsy to keep API contracts clean.- await workspaceService.searchEntity(workspaceSlug?.toString() ?? "", { - ...payload, - project_id: projectId, - }) + await workspaceService.searchEntity(workspaceSlug ?? "", { + ...payload, + ...(projectId ? { project_id: projectId } : {}), + })
229-231: Use console.error for failures and a clearer message.- console.log("Error in uploading asset:", error); + console.error("Error uploading editor asset:", error);
73-74: TNameDescriptionLoader supports "submitting" and "submitted" — types are correct.Confirmed: packages/types/src/common.ts exports
TNameDescriptionLoader = "submitting" | "submitted" | "saved". Optional: replace the string-union with an exported enum/const to avoid magic strings across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/core/components/editor/rich-text/description-input/index.ts(1 hunks)apps/web/core/components/editor/rich-text/description-input/loader.tsx(1 hunks)apps/web/core/components/editor/rich-text/description-input/root.tsx(4 hunks)apps/web/core/components/inbox/content/issue-root.tsx(3 hunks)apps/web/core/components/issues/issue-detail/main-content.tsx(2 hunks)apps/web/core/components/issues/peek-overview/issue-detail.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/core/components/editor/rich-text/description-input/loader.tsx (1)
packages/ui/src/loader.tsx (1)
Loader(30-30)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
DescriptionInput(88-241)
apps/web/core/components/issues/issue-detail/main-content.tsx (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
DescriptionInput(88-241)
apps/web/core/components/editor/rich-text/description-input/root.tsx (4)
packages/editor/src/core/types/extensions.ts (1)
TExtensions(1-1)packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-137)packages/utils/src/work-item/base.ts (1)
getDescriptionPlaceholderI18n(223-227)apps/web/core/components/editor/rich-text/description-input/loader.tsx (1)
DescriptionInputLoader(9-38)
apps/web/core/components/inbox/content/issue-root.tsx (2)
apps/web/core/components/editor/rich-text/description-input/loader.tsx (1)
DescriptionInputLoader(9-38)apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
DescriptionInput(88-241)
⏰ 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 and lint web apps
🔇 Additional comments (4)
apps/web/core/components/issues/issue-detail/main-content.tsx (1)
137-142: Await/return the update promise in onSubmit.Return (or await) the promise from issueOperations.update so the debounced autosave only marks "submitted" after persistence.
File: apps/web/core/components/issues/issue-detail/main-content.tsx Lines: 137-142
- onSubmit={async (value) => { + onSubmit={async (value) => { if (!issue.id || !issue.project_id) return; - issueOperations.update(workspaceSlug, issue.project_id, issue.id, { + return issueOperations.update(workspaceSlug, issue.project_id, issue.id, { description_html: value, }); - }} + }}Run to find other similar callsites (avoid process-substitution errors):
#!/bin/bash rg -l --hidden --no-ignore-vcs 'onSubmit=\{\s*async' -g '!node_modules/**' > /tmp/on_submit_files.txt || true rg -l --hidden --no-ignore-vcs 'issueOperations\.update\(' -g '!node_modules/**' > /tmp/update_files.txt || true sort /tmp/on_submit_files.txt -o /tmp/on_submit_files.txt sort /tmp/update_files.txt -o /tmp/update_files.txt comm -12 /tmp/on_submit_files.txt /tmp/update_files.txt || trueapps/web/core/components/inbox/content/issue-root.tsx (1)
153-153: Good change: escalate logging on failure.Switching to
console.errorimproves visibility in monitoring/log capture.apps/web/core/components/editor/rich-text/description-input/root.tsx (2)
84-89: Nice generalization and clear docstring.The componentization and prop hygiene look good.
45-46: No action required — RichTextEditor forwards EditorRefApi.
RichTextEditorWithRef (packages/editor) and apps/web's RichTextEditor are declared with forwardRef, so passing editorRef?: React.RefObject to ref is type-compatible.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
6-11: Nit: keep import grouping consistent (duplicate of prior feedback).Maintain consistent grouping of plane utils/components imports; this matches project conventions and reduces churn.
🧹 Nitpick comments (1)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
134-139: Optional: explicitly return the update promise for clarity/tests.Returning the awaited promise makes intent explicit and can help in unit tests that assert on the resolved value.
- await issueOperations.update(workspaceSlug, issue.project_id, issue.id, { + return await issueOperations.update(workspaceSlug, issue.project_id, issue.id, { description_html: value, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/core/components/editor/rich-text/description-input/loader.tsx(1 hunks)apps/web/core/components/inbox/content/issue-root.tsx(3 hunks)apps/web/core/components/issues/issue-detail/main-content.tsx(2 hunks)apps/web/core/components/issues/peek-overview/issue-detail.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/core/components/editor/rich-text/description-input/loader.tsx
- apps/web/core/components/issues/issue-detail/main-content.tsx
- apps/web/core/components/inbox/content/issue-root.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx (1)
DescriptionInput(88-241)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
127-143: LGTM: DescriptionInput integration and autosave flow are correct.Passing editorRef, entityId, fileAssetType, workspaceSlug/projectId, and wiring onSubmit now awaits the update—addressing the earlier “return/await” concern so autosave completion mirrors the actual update.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/core/components/editor/rich-text/description-input/root.tsx (2)
210-215: Avoid unnecessary toString()/fallback on a required string prop.- searchMentionCallback={async (payload) => - await workspaceService.searchEntity(workspaceSlug?.toString() ?? "", { + searchMentionCallback={async (payload) => + await workspaceService.searchEntity(workspaceSlug, { ...payload, project_id: projectId, }) }
231-233: Log upload errors with console.error (more accurate severity).- console.log("Error in uploading asset:", error); + console.error("Error in uploading asset:", error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/core/components/editor/rich-text/description-input/root.tsx (4)
packages/editor/src/core/types/extensions.ts (1)
TExtensions(1-1)packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-137)packages/utils/src/work-item/base.ts (1)
getDescriptionPlaceholderI18n(223-227)apps/web/core/components/editor/rich-text/description-input/loader.tsx (1)
DescriptionInputLoader(9-33)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/core/components/editor/rich-text/description-input/root.tsx (3)
105-108: Normalize description_html to a non-empty string and drop redundant fallbacks (prevents loader flicker).Keep description_html strictly a string (use "
" as empty) and remove the extra "?? ''" later.@@ const [localDescription, setLocalDescription] = useState<TFormData>({ id: entityId, - description_html: initialValue?.trim() ?? "", + description_html: + initialValue?.trim() === "" ? "<p></p>" : (initialValue ?? "<p></p>"), }); @@ const { handleSubmit, reset, control } = useForm<TFormData>({ defaultValues: { id: entityId, - description_html: initialValue?.trim() ?? "", + description_html: + initialValue?.trim() === "" ? "<p></p>" : (initialValue ?? "<p></p>"), }, }); @@ - initialValue={localDescription.description_html ?? "<p></p>"} + initialValue={localDescription.description_html}Optional: extract a small normalize helper to avoid repetition.
Also applies to: 119-124, 197-197
198-206: Verify SWR value won’t clobber local unsaved edits.Feeding value={swrDescription ?? null} may overwrite in-progress local changes if SWR revalidates mid-edit. Consider suppressing remote value while hasUnsavedChanges is true, or merging intelligently.
Would you confirm current RichTextEditor behavior with live SWR updates during typing and that local edits aren’t lost?
126-133: Fix stale onSubmit captured by debounce; only clear dirty flag on success.The debounced closure can call an outdated onSubmit, and finally() clears hasUnsavedChanges even on failure.
@@ - const handleDescriptionFormSubmit = useCallback( - async (formData: TFormData) => { - await onSubmit(formData.description_html); - }, - [onSubmit] - ); + const handleDescriptionFormSubmit = useCallback(async (formData: TFormData) => { + await onSubmitRef.current(formData.description_html); + }, []); @@ - const debouncedFormSave = useCallback( - debounce(async () => { - handleSubmit(handleDescriptionFormSubmit)() - .catch((error) => console.error(`Failed to save description for ${entityId}:`, error)) - .finally(() => { - setIsSubmitting("submitted"); - hasUnsavedChanges.current = false; - }); - }, 1500), - [entityId, handleSubmit] - ); + const debouncedFormSave = useCallback( + debounce(async () => { + handleSubmit(async ({ description_html }) => { + await onSubmitRef.current(description_html); + })() + .then(() => { + setIsSubmitting("submitted"); + hasUnsavedChanges.current = false; + }) + .catch((error) => { + console.error(`Failed to save description for ${entityId}:`, error); + // keep dirty so unmount or next change can retry + }); + }, 1500), + [entityId, handleSubmit] + );Add the ref to keep the latest submit handler:
// place above handleDescriptionFormSubmit const onSubmitRef = useRef(onSubmit); useEffect(() => { onSubmitRef.current = onSubmit; }, [onSubmit]);Also applies to: 152-162
| // Save on unmount if there are unsaved changes | ||
| useEffect( | ||
| () => () => { | ||
| debouncedFormSave.cancel(); | ||
|
|
||
| if (hasUnsavedChanges.current) { | ||
| handleSubmit(handleDescriptionFormSubmit)() | ||
| .catch((error) => { | ||
| console.error("Failed to save description on unmount:", error); | ||
| }) | ||
| .finally(() => { | ||
| setIsSubmitting("submitted"); | ||
| hasUnsavedChanges.current = false; | ||
| }); | ||
| } | ||
| }, | ||
| // since we don't want to save on unmount if there are no unsaved changes, no deps are needed | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [] | ||
| ); |
There was a problem hiding this comment.
Unmount save uses stale closures and cancels only the initial debounced function.
Tie cleanup to the latest debounced instance and use onSubmitRef; clear dirty only on success.
- useEffect(
- () => () => {
- debouncedFormSave.cancel();
-
- if (hasUnsavedChanges.current) {
- handleSubmit(handleDescriptionFormSubmit)()
- .catch((error) => {
- console.error("Failed to save description on unmount:", error);
- })
- .finally(() => {
- setIsSubmitting("submitted");
- hasUnsavedChanges.current = false;
- });
- }
- },
- // since we don't want to save on unmount if there are no unsaved changes, no deps are needed
- // eslint-disable-next-line react-hooks/exhaustive-deps
- []
- );
+ useEffect(() => {
+ return () => {
+ debouncedFormSave.cancel();
+ if (hasUnsavedChanges.current) {
+ handleSubmit(async ({ description_html }) => {
+ await onSubmitRef.current(description_html);
+ })()
+ .then(() => {
+ setIsSubmitting("submitted");
+ hasUnsavedChanges.current = false;
+ })
+ .catch((error) => {
+ console.error("Failed to save description on unmount:", error);
+ });
+ }
+ };
+ }, [debouncedFormSave, handleSubmit]);📝 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.
| // Save on unmount if there are unsaved changes | |
| useEffect( | |
| () => () => { | |
| debouncedFormSave.cancel(); | |
| if (hasUnsavedChanges.current) { | |
| handleSubmit(handleDescriptionFormSubmit)() | |
| .catch((error) => { | |
| console.error("Failed to save description on unmount:", error); | |
| }) | |
| .finally(() => { | |
| setIsSubmitting("submitted"); | |
| hasUnsavedChanges.current = false; | |
| }); | |
| } | |
| }, | |
| // since we don't want to save on unmount if there are no unsaved changes, no deps are needed | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| [] | |
| ); | |
| // Save on unmount if there are unsaved changes | |
| useEffect(() => { | |
| return () => { | |
| debouncedFormSave.cancel(); | |
| if (hasUnsavedChanges.current) { | |
| handleSubmit(async ({ description_html }) => { | |
| await onSubmitRef.current(description_html); | |
| })() | |
| .then(() => { | |
| setIsSubmitting("submitted"); | |
| hasUnsavedChanges.current = false; | |
| }) | |
| .catch((error) => { | |
| console.error("Failed to save description on unmount:", error); | |
| }); | |
| } | |
| }; | |
| }, [debouncedFormSave, handleSubmit]); |
🤖 Prompt for AI Agents
apps/web/core/components/editor/rich-text/description-input/root.tsx around
lines 164 to 183: the unmount cleanup currently cancels only the initial
debounced function and calls handleSubmit via a stale closure, and it clears the
dirty flag regardless of save success; fix by storing the latest
debouncedFormSave and submit handler in refs (e.g., debouncedFormSaveRef and
onSubmitRef), call debouncedFormSaveRef.current?.cancel() in the cleanup, invoke
the current submit function via onSubmitRef.current() (await it), and only set
hasUnsavedChanges.current = false inside the success branch (not in finally);
update relevant places to keep those refs in sync with the latest instances so
the cleanup uses the latest functions.
Description
This PR introduces a new, generic description input component. All the already existing feature specific components are now replaced with this.
Type of Change
Summary by CodeRabbit