Skip to content
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

Fix issue #4830: [Bug]: Copy-paste into the "What do you want to build?" bar doesn't work #4832

Merged
merged 8 commits into from
Nov 8, 2024

Conversation

openhands-agent
Copy link
Contributor

@openhands-agent openhands-agent commented Nov 7, 2024

This pull request fixes #4830.

The issue has been successfully resolved. The AI identified and fixed the core problem where text pasting wasn't working in the chat input bar. The solution involved:

Automatic fix generated by OpenHands 🙌


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:868c3c0-nikolaik   --name openhands-app-868c3c0   docker.all-hands.dev/all-hands-ai/openhands:868c3c0

Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a perfect fix to me, amazing. But @amanape could you double-check?

BTW: I ran locally before and after and confirmed that this did indeed fix the bug.

@neubig neubig requested a review from amanape November 7, 2024 18:19
@neubig neubig marked this pull request as ready for review November 7, 2024 18:21
@amanape
Copy link
Member

amanape commented Nov 7, 2024

Oh I see. @xingyaoww's chat input PR did this but I changed the position of the event.preventDefault() incorrectly. Looks like it caused it to fail and OpenHands fixed it again!

The only thing that OpenHands missed by a tiny bit are the tests (though I'm more than glad of the fact that it has attempted to do so). We have a test files for the chat input:

https://github.com/All-Hands-AI/OpenHands/blob/main/frontend/__tests__/components/chat/chat-input.test.tsx

Instead, OpenHands created a new file.

Some events it utilizes could be improved if it read the previous tests (such as using userEvent over fireEvent).

BTW, is OpenHands instructed to write tests?

@neubig neubig added the fix-me Attempt to fix this issue with OpenHands label Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor Author

New OpenHands update

Copy link
Contributor

github-actions bot commented Nov 7, 2024

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

@amanape
Copy link
Member

amanape commented Nov 7, 2024

Revert these commits. I noticed that it started over in the trajectory, I didn't seem to find my feedback there. It worked off the original issue it seems

Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The tests should be added to the already-existing file: https://github.com/All-Hands-AI/OpenHands/blob/main/frontend/__tests__/components/chat/chat-input.test.tsx
  2. Some lint rules are failing in frontend/src/components/chat-input.tsx

@neubig neubig added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor Author

New OpenHands update

Copy link
Contributor

github-actions bot commented Nov 7, 2024

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

@neubig neubig force-pushed the openhands-fix-issue-4830 branch from 9987ff4 to 9e53ff7 Compare November 7, 2024 23:24
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and I think that it reflects @amanape's comments as well.

Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing, we may need to use fireEvent instead of userEvent for just the case of copy-pasting images.

@neubig neubig added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

OpenHands started fixing the pr! You can monitor the progress here.

@neubig neubig added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

OpenHands started fixing the pr! You can monitor the progress here.

Copy link
Contributor

github-actions bot commented Nov 8, 2024

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

@openhands-agent
Copy link
Contributor Author

New OpenHands update

Copy link
Contributor

github-actions bot commented Nov 8, 2024

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

@neubig neubig force-pushed the openhands-fix-issue-4830 branch from 2971dd2 to 654ade8 Compare November 8, 2024 01:49
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good now. I wasn't able to get tests working properly with userEvent, but if it's important to do so maybe we can do so in a follow-up PR.

@neubig neubig enabled auto-merge (squash) November 8, 2024 02:05
@neubig neubig disabled auto-merge November 8, 2024 02:19
@neubig neubig merged commit f5003a7 into main Nov 8, 2024
8 checks passed
@neubig neubig deleted the openhands-fix-issue-4830 branch November 8, 2024 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix-me Attempt to fix this issue with OpenHands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Copy-paste into the "What do you want to build?" bar doesn't work
3 participants