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

Show screenshot #11

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

Conversation

li-boxuan
Copy link

Demo:

Screenshot 2025-03-20 at 12 38 57 AM

Copy link

vercel bot commented Mar 20, 2025

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

Name Status Preview Comments Updated (UTC)
trajectory-visualizer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2025 11:47pm

Copy link
Contributor

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

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

LGTM! Sorry it must have been the agent 😢

@li-boxuan
Copy link
Author

li-boxuan commented Mar 28, 2025

LGTM! Sorry it must have been the agent 😢

haha yeah but just so you know, I didn't fix it in this PR coz I was too lazy. This PR is based on the old version.

@xingyaoww
Copy link
Contributor

@OpenHands this PR is related to #13 -- can you look into it and see why it failed -- and what should we do to fix it?

Copy link

openhands-ai bot commented Mar 28, 2025

I'm on it! @xingyaoww can track my progress at all-hands.dev

Copy link

openhands-ai bot commented Mar 28, 2025

Summary of Work

I investigated the issue with PR #11 (add-screenshot) and its relation to issue #13 (timeline entries not displaying properly).

Root Cause Analysis

The problem was caused by a conflict between two versions of the TimelineStep.tsx component:

  1. PR Add JSONL evaluation viewer with instance navigation #10 (JSONL evaluation viewer) had moved the TimelineStep component to a components subdirectory
  2. The original TimelineStep.tsx file remained in the root timeline directory
  3. PR Show screenshot #11 was trying to add screenshot support to the old file, which was no longer being used

Solution Implemented

I created PR #15 with the following changes:

  1. Removed the old TimelineStep.tsx file from the root timeline directory
  2. Added the screenshot support to the TimelineStep component in the components directory
  3. Preserved all the functionality from both components

Results

The fix should resolve the issue where timeline entries weren't displaying properly. By removing the duplicate component and adding the screenshot support to the correct component, the timeline entries should now display correctly with screenshot support.

The PR has been created and is ready for review: #15

@neubig
Copy link
Contributor

neubig commented Apr 3, 2025

@OpenHands resolve merge conflicts on this PR

Copy link

openhands-ai bot commented Apr 3, 2025

I'm on it! @neubig can track my progress at all-hands.dev

@li-boxuan
Copy link
Author

li-boxuan commented Apr 5, 2025

Demo (with set of marks)

Screenshot 2025-04-05 at 4 47 24 PM

traj_sde-create-new-gitlab-project-logo-image.json

@li-boxuan
Copy link
Author

@xingyaoww @neubig can I get an approval for the workflow please?

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