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 export conversation button in Safari #7662

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

csmith49
Copy link
Collaborator

@csmith49 csmith49 commented Apr 2, 2025

Description

This PR fixes the issue with the "export conversation" button not working in Safari browsers.

Problem

The current implementation uses the File System Access API (showSaveFilePicker), which is not supported in Safari. This causes the export functionality to fail silently in Safari browsers.

Solution

Implemented a fallback mechanism that uses the traditional download method (downloadJSON) when the File System Access API is not available. This ensures that the export functionality works across all browsers, including Safari.

Changes

  • Modified downloadTrajectory function to check for browser support and use appropriate download method
  • Added fallback to use downloadJSON for browsers that do not support File System Access API
  • Added error handling to fall back to traditional download method if the user cancels the save dialog or any other error occurs

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:61c155a-nikolaik   --name openhands-app-61c155a   docker.all-hands.dev/all-hands-ai/openhands:61c155a

Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

Thank you for this, it's been bugging me!

Can't wait to try it. 🥳

@openhands-agent
Copy link
Contributor

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

@enyst
Copy link
Collaborator

enyst commented Apr 14, 2025

Do you think we can merge this?

We have a report on the button doing nothing, though I'm not sure what browser were they using.

@csmith49
Copy link
Collaborator Author

Do you think we can merge this?

We have a report on the button doing nothing, though I'm not sure what browser were they using.

Yeah, should be able to merge it. Was stuck with some issues with the dummy agent tests I couldn't sort out (but those are no longer blocking).

@enyst
Copy link
Collaborator

enyst commented Apr 14, 2025

Ah yes, and we're removing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants