Skip to content

fix(studio): Fileset panel animations#441

Open
steramae-nvidia wants to merge 2 commits into
mainfrom
steramae/panel-animation
Open

fix(studio): Fileset panel animations#441
steramae-nvidia wants to merge 2 commits into
mainfrom
steramae/panel-animation

Conversation

@steramae-nvidia

@steramae-nvidia steramae-nvidia commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
Screen.Recording.2026-06-24.at.12.04.59.PM.mov

Summary by CodeRabbit

  • Bug Fixes
    • Improved file preview and dataset side panel closing so dismissals complete more reliably before navigating to other views.
    • Fixed breadcrumb navigation in the file preview panel to handle deferred navigation correctly, reducing missed clicks during folder/fileset transitions.
    • Enhanced outside-click and escape-key dismissal behavior for preview panels for more consistent panel closing.

Signed-off-by: Sean Teramae <steramae@nvidia.com>
@steramae-nvidia steramae-nvidia requested review from a team as code owners June 24, 2026 19:06
@github-actions github-actions Bot added the fix label Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7dfbcedc-7d67-41ea-a1b3-84cfcfc5526e

📥 Commits

Reviewing files that changed from the base of the PR and between 35ed320 and b44b3a7.

📒 Files selected for processing (2)
  • web/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsx
  • web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsx
  • web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx

📝 Walkthrough

Walkthrough

Fileset preview close handling now queues breadcrumb navigation through the panel close lifecycle, adds an onClosing callback, updates route panel state from URL params, and changes the tests to wait for deferred callbacks.

Changes

Fileset preview panel closing

Layer / File(s) Summary
Preview close orchestration
web/packages/studio/src/components/FilesetFilePreviewPanel/index.tsx, web/packages/studio/src/components/filesets/FilesetSidePanelWrapper/index.tsx, web/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsx
FilesetFilePreviewPanel adds onClosing, queues breadcrumb navigation through the close button, renders the close button explicitly, and the tests wait for the delayed breadcrumb callbacks.
Panel route state wiring
web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx
PanelManagement derives panel open state from URL params, closes panels directly, and passes onClosing into FilesetFilePreviewPanel.

Suggested labels

fix

Suggested reviewers

  • htolentino-nvidia
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing Studio Fileset panel animations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch steramae/panel-animation

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsx`:
- Around line 225-230: The breadcrumb and folder-click tests in the
FilesetFilePreviewPanel suite only verify eventual callback execution, so they
can miss regressions where navigation happens synchronously. Update the affected
tests around onFolderClick and the folder breadcrumb clicks to assert
immediately after each fireEvent.click that the callback has not yet been
called, then keep the existing awaited expectation to confirm it fires later.
Apply this pattern in the relevant test cases that currently rely on waitFor
alone.

In `@web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx`:
- Around line 47-49: The dataset side panel is being unmounted too early because
showDatasetPanel is tied directly to datasetIdFromUrl, and the close handler in
PanelManagement removes that URL before the exit animation can run. Update the
panel state flow so DatasetFileManagementSidePanel stays mounted with
open={false} while closing, using a separate closing dataset id or deferring the
URL cleanup until close-complete. Make sure the logic around showDatasetPanel,
the close handler, and the DatasetFileManagementSidePanel open/close wiring all
work together to preserve the panel during animation.
- Around line 43-44: Stop decoding the route params in PanelManagement:
useParams() already provides decoded values and getFilesetFileRoute() handles
encoding, so the extra decodeURIComponent calls on datasetIdFromUrl and
filePathFromUrl can throw on valid names containing %. Remove the redundant
decoding in the PanelManagement component and use the params directly where
datasetIdFromUrl and filePathFromUrl are assigned.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6c4a7571-da21-44ce-929f-8fbad0037304

📥 Commits

Reviewing files that changed from the base of the PR and between 8c3a0c0 and 35ed320.

📒 Files selected for processing (4)
  • web/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsx
  • web/packages/studio/src/components/FilesetFilePreviewPanel/index.tsx
  • web/packages/studio/src/components/filesets/FilesetSidePanelWrapper/index.tsx
  • web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx

Comment thread web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx Outdated
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 20908/27478 76.1% 61.2%
Integration Tests 12108/26247 46.1% 19.5%

Signed-off-by: Sean Teramae <steramae@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant