Comments cannot be edited or deleted

[12joan]

^{Originally posted by **wenyongqd** August 7, 2023} Want to know why I can not edit a comment and how can I make it works?

Description Comments on platejs.org cannot be edited or deleted, although they can be created and marked as resolved.

Steps

1. Go to the playground
2. Click inside a comment
3. Press the edit or delete buttons
4. Nothing happens

Demo

https://github.com/udecode/plate/assets/4272090/fdbbd2df-bead-440c-93fc-65e59d72d458

Funding

• You can sponsor this specific effort via a Polar.sh pledge below
• We receive the pledge once the issue is completed & verified

[woodpeng]

Facing same issue. Any solutions?

[EandrewJones]

Also facing this issue. Will circle back to it after I close other tickets in our project. If this still isn't resolved, I'll try to fix it and create a PR.

[EandrewJones]

Okay. I looked into this, identified the cause, and found a fix. However, there are a few options for how and where to fix.

Cause

The hook for returning the button props does not directly return the props, instead it returns an object with a props property. This

packages/comments/src/components/CommentEditButton.tsx

export const useCommentEditButton = ({
  setIsMenuOpen,
  comment,
  editingValue,
}: ReturnType<typeof useCommentEditButtonState>) => {
  return {
    props: {
      onClick: () => {
        setIsMenuOpen(false);
        editingValue(comment.value);
      },
    },
  };
};

In templates/plate-playground-template/src/components/plate-ui/comment-more-dropdown.tsx, the object is directly assigned rather than unpacking the props field. The type casting of editProps and deleteProps to any is the smelly giveaway you have an issue here. If they were proper props objects, there's no type error.

export function CommentMoreDropdown() {
  const editButtonState = useCommentEditButtonState();
  const editProps = useCommentEditButton(editButtonState);
  const deleteButtonState = useCommentDeleteButtonState();
  const deleteProps = useCommentDeleteButton(deleteButtonState);

  return (
    <DropdownMenu modal={false}>
      <DropdownMenuTrigger asChild>
        <Button variant="ghost" className={cn('h-6 p-1 text-muted-foreground')}>
          <Icons.more className="h-4 w-4" />
        </Button>
      </DropdownMenuTrigger>
      <DropdownMenuContent>
        <DropdownMenuItem {...(editProps as any)}>
          Edit comment
        </DropdownMenuItem>
        <DropdownMenuItem {...(deleteProps as any)}>
          Delete comment
        </DropdownMenuItem>
      </DropdownMenuContent>
    </DropdownMenu>
  );
}

Fix

The fix is easy, pass the correct props to the DropdownMenuItem. However, you have two options:

A. Unpack the props field in comment-more-dropdown.tsx or directly reference the property

...
    const editButtonState = useCommentEditButtonState()
    const { props: editProps } = useCommentEditButton(editButtonState)
    const deleteButtonState = useCommentDeleteButtonState()
    const { props: deleteProps } = useCommentDeleteButton(deleteButtonState)
...
    // simple spread
    <DropdownMenuItem {...editProps}>Edit comment</DropdownMenuItem>
...
    // Or reference
    const editButtonState = useCommentEditButtonState()
    const editButton = useCommentEditButton(editButtonState)
    const deleteButtonState = useCommentDeleteButtonState()
    const deleteButton = useCommentDeleteButton(deleteButtonState)

    <DropdownMenuItem {...editButton.props}>Edit comment</DropdownMenuItem>

or B. update the useCommentEditButton to directly return the props object instead of placing it into an object.

export const useCommentEditButton = ({
    setIsMenuOpen,
    comment,
    editingValue,
}: ReturnType<typeof useCommentEditButtonState>) => {
    return {
        onClick: () => {
            console.log('clicked')
            setIsMenuOpen(false)
            editingValue(comment.value)
        },
    }
}

The answer comes to maintainer's choice. Are you intentionally enforcing a design pattern on the use hooks where props should be a field within the object because you intend to extend this object in the future or is it just convention by happenstance? I'm not sure how many other places in the code base are impacted by the nested props field problem. Option B is probably more maintainable long-term because you don't need to unpack props or refernce the field, you can just pass it directly to the component.

@zbeyens You know the code base better. Name it and I'll implement it.

Knock-on Problem

The above fix exposes a further problem in the CommentValue component.

export function CommentValue() {
  const { textareaRef } = useCommentValue();

  return (
    <div className="my-2 flex flex-col items-end gap-2">
      <CommentEditTextarea
        ref={textareaRef}
        className={cn(inputVariants(), 'min-h-[60px]')}
      />

      <div className="flex space-x-2">
        <CommentEditActions.CancelButton
          className={buttonVariants({ variant: 'outline', size: 'xs' })}
        >
          Cancel
        </CommentEditActions.CancelButton>

        <CommentEditActions.SaveButton
          className={buttonVariants({ variant: 'default', size: 'xs' })}
        >
          Save
        </CommentEditActions.SaveButton>
      </div>
    </div>
  );
}

The component throws an error because useCommentValue. It creates a ref that never gets updated. I'm not quite sure what it's trying to achieve.

However, the easy fix is to use the state/prop hooks for CommentEditTextarea:

export function CommentValue() {
    const commentTextAreaState = useCommentEditTextareaState()
    const { props: textareaProps } =
        useCommentEditTextarea(commentTextAreaState)
    // const { textareaRef } = useCommentValue()
    // console.log({ textareaRef })

    return (
        <div className="my-2 flex flex-col items-end gap-2">
            <CommentEditTextarea
                {...textareaProps}
                // ref={textareaRef}
                className={cn(inputVariants(), 'min-h-[60px]')}
            />

            <div className="flex space-x-2">
                <CommentEditActions.CancelButton
                    className={buttonVariants({
                        variant: 'outline',
                        size: 'xs',
                    })}
                >
                    Cancel
                </CommentEditActions.CancelButton>

                <CommentEditActions.SaveButton
                    className={buttonVariants({
                        variant: 'default',
                        size: 'xs',
                    })}
                >
                    Save
                </CommentEditActions.SaveButton>
            </div>
        </div>
    )
}

Again notice the need to unpack or reference props when passing to the component..

Do we need useCommentValue here? If so, can you explain the intended effect.

Let me know and I'll get the PR in ASAP (it's a blocker for us so might as well push upstream too).

[12joan]

@EandrewJones Nice work on this!

For the props objects, let's follow the pattern in link-element.tsx and destructure it imediately after calling. That's the first pattern in your solution A.

Let me take a look at that ref object...

[EandrewJones]

Love the immediate reply, Joe. Will try to push the PR before COB pending response on useCommentValue.

Best

Evan Jones Website: www.ea-jones.com

On Fri, Nov 3, 2023 at 12:12 PM Joe Anderson @.***> wrote:

@EandrewJones https://github.com/EandrewJones Nice work on this!

For the props objects, let's follow the pattern in link-element.tsx and destructure it imediately after calling. That's the first pattern in your solution A.

Let me take a look at that ref object...

— Reply to this email directly, view it on GitHub https://github.com/udecode/plate/issues/2563#issuecomment-1792728021, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ2T6AJMAZFZ2DLWEVQFVHDYCUJYRAVCNFSM6AAAAAA3HLQYVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJSG4ZDQMBSGE . You are receiving this because you were mentioned.Message ID: @.***>

[12joan]

It looks like the intended effect of the useCommentValue ref is to automatically select all text when the textarea mounts, but this seems to be already handled in CommentEditTextarea. If you remove the ref from comment-value.tsx, does the autofocus work as expected?

[12joan]

Also, just to clarify since it isn't immediately obvious, the files in templates/plate-playground-template are generated automatically from those in apps/www/src/registry; it's the latter files you want to edit in your PR.

[EandrewJones]

1. CommentEditTextarea autofocuses but does not select the text.
2. Re-adding the useCommentValue ref still throws. Looks like there's a bug in it to me

    const textarea = textareaRef.current!;

Another type assertion smell. When the component first mounts, the ref is null so textarea is null. Then it tries to destructure properties like length from the value which don't yet exist... Indeed, ref is never assigned a value even after the CommendEditTextarea mounts or it at least it looks that way:

export const useCommentValue = () => {
    const textareaRef = useRef<HTMLTextAreaElement>(null)

    useEffect(() => {
        const textarea = textareaRef.current
        if (textarea) {
            console.log(textarea)
            textarea.focus()

            const { length } = textarea.value
            textarea.setSelectionRange(length, length)
        }
    }, [textareaRef])

    return {
        textareaRef,
    }
}

The above stops the hook from throwing, but textarea is never updated so the selection never occurs.

re: generated automatically from those in apps/www/src/registry; TY for clarifying.

[12joan]

I think autofocusing but not selecting is fine for now. Most users who edit a comment likely want to fix a typo or add details, not replace the entire text. Can you remove the useCommentValue hook entirely? This will be a breaking change, so your PR changeset should be a major release.

[EandrewJones]

I think autofocusing but not selecting is fine for now

Agreed.

This will be a breaking change, so your PR changeset should be a major release.

:+1: