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

feat(ui): collapsible sidebar #35018

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Mar 4, 2025

Screen.Recording.2025-03-04.at.15.17.02.mov

@Skn0tt Skn0tt self-assigned this Mar 4, 2025
@Skn0tt Skn0tt requested a review from Copilot March 4, 2025 14:18

This comment has been minimized.

Choose a reason for hiding this comment

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

PR Overview

This PR introduces a collapsible sidebar feature to the UI by adding expand/collapse functionality and updating both the UI test suite and the underlying UI view components.

  • Added an end-to-end test in the Playwright suite to verify the sidebar’s collapsed state and test-run behavior.
  • Updated the UIModeView to support toggling between expanded and collapsed sidebar states.
  • Adjusted the SplitView component styling to respect the sidebarHidden property.

Reviewed Changes

File Description
tests/playwright-test/ui-mode-test-run.spec.ts Added a new test to verify running tests from a collapsed sidebar
packages/trace-viewer/src/ui/uiModeView.tsx Integrated state and UI controls for toggling the sidebar and updated layout classes
packages/web/src/components/splitView.tsx Refactored sidebar rendering to consistently apply visibility styles

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

tests/playwright-test/ui-mode-test-run.spec.ts:810

  • [nitpick] Using an exact multiline string for asserting the UI state can be brittle, especially if whitespace or formatting changes. Consider using a regex or normalizing the whitespace before comparison to make the test more robust.
  await expect.poll(dumpTestTree(page)).toBe(`
    ▼ ❌ a.test.ts
        ✅ passes
        ❌ fails <=
      ► ❌ suite
    ▼ ❌ b.test.ts
        ✅ passes
        ❌ fails
    ▼ ✅ c.test.ts
        ✅ passes
        ⊘ skipped
  `);

packages/trace-viewer/src/ui/uiModeView.tsx:404

  • Changing the layout container class from 'vbox' to 'hbox' alters the flex direction and may affect the overall UI layout. Please ensure the associated CSS rules are updated to support the intended horizontal layout behavior.
  return <LLMProvider><div className='hbox ui-mode'>

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Mar 4, 2025

Test results for "tests 1"

1 failed
❌ [default-reuse] › tests/run-tests.spec.ts:340:5 › should only create test run if file belongs to context @vscode-extension

2 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [playwright-test] › tests/ui-mode-test-watch.spec.ts:145:5 › should watch all @windows-latest-node18-1

34612 passed, 708 skipped
✔️✔️✔️

Merge workflow run.

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.

1 participant