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 #5965: [Bug]: Misc UI issues due to resizable and collapsible panel #5966

Closed
wants to merge 21 commits into from

Conversation

openhands-agent
Copy link
Contributor

@openhands-agent openhands-agent commented Jan 2, 2025

This PR fixes an issue with the resizable panel component where the right panel would overflow outside the browser border when the left chat panel is expanded to its maximum width (50%).

Changes made:

  • Changed right panel width constraints from fixed pixels to proportional values:
    • minWidth changed from 600px to 30%
    • Added maxWidth: "70%" to ensure the panel stays within bounds
  • Keeps existing left panel constraint of maxWidth: "50%"

These changes ensure:

  1. The right panel maintains a reasonable size relative to the window width
  2. The panels never overflow outside the browser window
  3. The layout remains responsive across different screen sizes

The changes have been tested locally and all tests pass.


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

@xingyaoww
Copy link
Collaborator

The code changes so far do fix the issue when I open OpenHands in a weird aspect ratio screen that I used to do.

image

It looks good on a large screen.

image

However, I'm still able to drag each panel to be very small and make it doesn't look good.

image

image

@openhands-agent Can you look into it?

@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Status Overview:

✅ Progress Made:

  • Implemented minimum/maximum constraints
  • Set default ratios

❌ Remaining Issues:

  1. Chat panel can still be compressed to unusable width
  2. Terminal can still be resized too small

Recommended Next Steps:

  1. Verify constraint implementation
  2. Increase minimum size thresholds
  3. Test constraint enforcement

Summary: While initial improvements were made, the core usability issues persist. Further adjustments to the implementation are needed to fully resolve the panel sizing problems.

@xingyaoww
Copy link
Collaborator

You should first read the "diff" against main to understand existing implementation, then check the history of this thread.

@xingyaoww xingyaoww added the fix-me Attempt to fix this issue with OpenHands label Jan 2, 2025
@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Concise Overview:

Status: PARTIALLY RESOLVED with remaining issues

Completed:
✓ Implementation of size constraints
✓ Set default ratios

Remaining Issues:

  • Panels can still be resized to unusably small dimensions despite constraints
  • Visual appearance is compromised at certain sizes

Next Steps Required:

  1. Debug constraint enforcement
  2. Validate constraint implementation
  3. Conduct comprehensive testing across screen sizes

Recommendation: Issue should remain open until panel sizing behavior is fully fixed and verified.

@xingyaoww
Copy link
Collaborator

OpenHands you need to run npm run lint:fix to fix linter error

@xingyaoww xingyaoww added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Jan 3, 2025
@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Summary of Issue Status:

UNRESOLVED - Panel Sizing Issues

Current State:

  • Changes were made to address panel sizing
  • No visual confirmation provided
  • Previous feedback indicated persistent issues
  • Lack of verification testing

Required for Resolution:

  1. Visual proof of proper panel sizing
  2. Multi-screen size layout testing
  3. Constraint validation testing
  4. Reviewer verification

Recommendation:
Request visual evidence and testing confirmation before marking as resolved.

@xingyaoww
Copy link
Collaborator

OpenHands -- resize the box will actually change the min-width. The current setup doesn't really prevent the panel from being resized too much. Can you help?

@xingyaoww xingyaoww added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Jan 3, 2025
@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Status Summary:

UNRESOLVED - Panel Sizing Issues

Key Points:

  • Changes were proposed but not confirmed to work
  • Missing crucial verification:
    • No visual proof
    • No reviewer approval
    • No confirmation of min-width fix
  • Historical context shows previous failed attempts

Recommendation:
Keep issue open pending:

  1. Implementation verification
  2. Reviewer confirmation
  3. Specific proof that min-width constraint works as intended

@xingyaoww
Copy link
Collaborator

ok OpenHands - this looks MUCH better!

Now there are several issues:

  1. The right side seems to go out of the browser window

image

  1. When we try to make the terminal panel larger, it didn't resize the terminal with the panel

image

  1. Minor nit: I want the left chat panel to max at max-width 50%, right now the max is a little small.

@xingyaoww xingyaoww added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Jan 3, 2025
@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Status Overview:

Pending Issues:

  • Right side panels extending beyond browser window
  • Terminal panel resizing problems
  • Chat panel max-width adjustment to 50%

Current State:
While changes have been implemented, there is no reviewer confirmation that the fixes are working as intended. Multiple previous attempts to resolve these issues were unsuccessful.

Recommendation:
Await reviewer verification before considering these issues closed. Visual confirmation is needed to ensure:

  1. Panel containment within browser
  2. Proper terminal resizing
  3. Correct chat panel width

Overall Status: UNRESOLVED - Pending reviewer confirmation

@xingyaoww
Copy link
Collaborator

xingyaoww commented Jan 3, 2025

The terminal is a little better but it still didn't fill all the pane space.

image

The right side of the panel still goes beyond the edge of the browser window.

image

@xingyaoww xingyaoww added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Jan 3, 2025
@openhands-agent
Copy link
Contributor Author

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

@openhands-agent
Copy link
Contributor Author

Status Overview:

❌ UNRESOLVED ISSUES:

  1. Terminal not filling allocated space
  2. Right panel extending beyond browser window

NEXT STEPS REQUIRED:

  • Implement proper terminal scaling
  • Add boundary constraints for right panel
  • Provide visual proof of fixes

SUMMARY:
The attempted fixes were unsuccessful, with core functionality issues still present. Visual evidence confirms the problems persist. Further development work needed before requirements can be considered met.

xingyaoww and others added 2 commits January 4, 2025 16:54
- Fix right panel extending beyond browser window
- Fix terminal panel not filling available space
- Adjust chat panel max width to 50%
- Simplify resize logic using React state
- Add proper flex properties for responsive behavior
@xingyaoww xingyaoww marked this pull request as ready for review January 4, 2025 22:45
xingyaoww and others added 4 commits January 6, 2025 16:40
- Remove window resize handling since panels use flex layout
- Use fixed min/max dimensions instead of window-based constraints
- Keep proportional sizing for second panel (30-70%)
@xingyaoww xingyaoww force-pushed the openhands-fix-issue-5965 branch from da96e04 to dcb24d3 Compare January 7, 2025 17:23
@xingyaoww
Copy link
Collaborator

Can't get this to work well 😓 gonna close this for now

@xingyaoww xingyaoww closed this Jan 17, 2025
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.

2 participants