fix: Comments cannot be edited or deleted

[EandrewJones]

Description

See changesets.

Closes #2563 and #2561

See #2563 for in-depth discussion of the proposed and decided upon fix. In brief, the props for the edit/delete buttons were not properly destructured and therefore the wrong item was being unpacked as props. In the comment editing form component, the state and props hooks needed to be added and passed to the text area.

[changeset-bot]

⚠️ No Changeset found

Latest commit: d3ee34d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
plate	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 5, 2023 5:11pm

[EandrewJones]

@12joan Which packages should I select for the changeset when none of have been modified? This is all components side.

Also happy to add a test to verify expected behavior but I'm having trouble tracing where existing tests for comments components are, if any, and I only see a single e2e test file for tables.

[reviewpad]

Thank you @EandrewJones for this first contribution!

[12joan]

@12joan Which packages should I select for the changeset when none of have been modified? This is all components side.

If there are genuinely no changes to any versioned packages, we can merge without a changeset. But I think useCommentValue is now unused, so we can delete that and make a changeset on the comments package.

Also happy to add a test to verify expected behavior but I'm having trouble tracing where existing tests for comments components are, if any, and I only see a single e2e test file for tables.

We didn't get very far with adding e2e tests, that's right. 🙂

If you want to try adding an e2e test, go ahead. It would certianly help to prevent this issue happening again, but it's not required to get the PR merged.

[EandrewJones]

We didn't get very far with adding e2e tests, that's right.

Totally understood. I can strongly relate.

Okay, I believe I've made all requested changes.