refactor: reuse SheetNavigationButtons component and useSheetNavigation hook

[impoiler]

Summary

Extracts the navigation button UI and keyboard shortcut logic from MCPLogDetailSheet into shared SheetNavigationButtons component and useSheetNavigation hook, reducing duplication and making sheet navigation reusable across other sheet views.

Changes

• Replaced the inline prev/next Button elements in MCPLogDetailSheet with the shared SheetNavigationButtons component
• Replaced direct useHotkeys calls with the useSheetNavigation hook, which returns the key bindings passed into SheetNavigationButtons for tooltip display
• Removed ChevronDown, ChevronUp icon imports and useHotkeys import from this file since they are now handled internally by the shared components

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

1. Open the MCP Logs view and click on a log entry to open the detail sheet.
2. Verify the previous/next navigation buttons appear and are enabled/disabled correctly based on position in the list.
3. Press the up/down arrow keys and confirm navigation between log entries works as expected.
4. Confirm keyboard shortcut hints are visible in the navigation button tooltips.

Screenshots/Recordings

N/A

Breaking changes

• No

Related issues

N/A

Security considerations

None.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d29c25b1-e2ce-4caf-8bcb-ab69a21caef7

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc25a8 and 05a4995.

📒 Files selected for processing (2)

• ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx
• ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx

🚧 Files skipped from review as they are similar to previous changes (2)

• ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx
• ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added confirmation prompts when navigating away from MCP client configuration sheets with unsaved changes, preventing accidental data loss.

• Improvements

  • Refined navigation handling in MCP logs and client views for better consistency and user experience.

Walkthrough

Two MCP workspace sheet components: the log details sheet is refactored to use a shared navigation hook and buttons, and the client sheet defers navigation when unsaved changes exist and shows a confirmation dialog to confirm discard before navigating.

Changes

Sheet Navigation UI Updates

Layer / File(s)	Summary
MCP Log Details Sheet Navigation Refactoring ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx	Replaces hotkey-based up/down handlers and chevron buttons with useSheetNavigation and SheetNavigationButtons; imports updated and prev/next key bindings plus onNavigate are wired into the sheet header.
MCP Client Sheet Unsaved Changes Protection ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx	Adds useCallback and derives isDirty from form and local state; navigation is memoized to store a pendingNavDirection when dirty, useSheetNavigation is disabled while pending, the component return is wrapped to render an AlertDialog alongside the sheet, and the dialog clears pending direction then invokes stored navigation.

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Possibly related PRs

  • maximhq/bifrost#3745: Refactors other sheets to use useSheetNavigation and SheetNavigationButtons (similar navigation unification).
  • maximhq/bifrost#3739: Introduced the shared useSheetNavigation/SheetNavigationButtons changes referenced by the log details update.

• Suggested reviewers

  • akshaydeo

"I hopped through the code with a twitchy nose,
Tabs and arrows now in neat little rows.
One sheet skips keys, one guards what you've penned,
A rabbit's small cheer for a safer end. 🐇✨"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main refactoring work: extracting shared navigation components and hooks into reusable pieces.
Description check	✅ Passed	The description covers all key sections with summary, changes, type, affected areas, testing steps, and related metadata appropriately filled in.
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 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[impoiler]

• feat: replace enabled badge with toggle switch and add edit action in MCP clients table #3809
• fix: exclude sonner toast elements from sheet outside-click detection #3805
• refactor: reuse SheetNavigationButtons component and useSheetNavigation hook #3804 👈 (View in Graphite)
• refactor: extract dashboard tab data-fetching into dedicated TabView components #3797
• feat: add GetDimensionRankings API and dashboard tabs for team, customer, BU, and user rankings #3766
• refactor: replace TeamDialog with TeamSheet and improve teams table layout #3757
• feat: add SearchSelect component with sync and async support #3752
• fix: persist customers pagination/search in URL state and improve table layout #3751
• feat: add prev/next navigation to RoutingRuleInfoSheet #3745
• feat: add prev/next navigation to MCPClientSheet #3744
• feat: add keyboard navigation and URL state for virtual key detail sheet #3740
• feat: add useSheetNavigation hook and SheetNavigationButtons component #3739
• refactor: replace CustomerDialog with CustomerSheet for customer create/edit #3726
• feat: log database type when connecting in NewConfigStore #3724
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

Safe to merge — this is a clean refactor with no functional regressions and one incidental improvement to navigation guard logic.

Both changed files correctly delegate to the shared components without altering any business logic. The enabled change in mcpClientSheet.tsx is an improvement over the original, and the AlertDialog restructuring is architecturally sound.

No files require special attention.

Important Files Changed

Filename	Overview
ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx	Replaces inline prev/next buttons and direct useHotkeys calls with the shared SheetNavigationButtons component and useSheetNavigation hook. Keyboard shortcuts now also include vim-style k/j bindings (added by the shared hook). No functional regressions.
ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx	Wraps Sheet + AlertDialog in a Fragment (moving the dialog outside the sheet for cleaner portal stacking), inlines confirmNavigation/cancelNavigation helpers, wraps handleNavigate in useCallback, and changes the navigation enabled condition from !!onNavigate to !pendingNavDirection. The new condition is actually an improvement: it prevents hotkeys from firing while the unsaved-changes confirm dialog is open, fixing a subtle race where pressing a key during the dialog would silently overwrite the pending direction.

_{Reviews (5): Last reviewed commit: "refactor: use sheetNavigation hook in mc..." | Re-trigger Greptile}

[coderabbitai]

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 `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx`:
- Around line 127-134: The SheetNavigationButtons usage in
mcpLogDetailsSheet.tsx lacks E2E selectors; update the SheetNavigationButtons
call to pass stable test id props (e.g., prevButtonTestId and nextButtonTestId
or a sheetTestId prefix) and then modify the SheetNavigationButtons component to
accept those props and apply them to the actual button elements (e.g., <Button
data-testid={prevButtonTestId}> and <Button data-testid={nextButtonTestId}>).
Ensure the prop names match between the call site and the component (referencing
SheetNavigationButtons, prevKeys, nextKeys, and onNavigate) so E2E tests can
target the prev/next controls reliably.

In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 1311-1327: Add stable E2E selectors to the new unsaved-navigation
dialog by adding data-testid attributes to the interactive controls: add
data-testid="unsaved-nav-cancel" to the AlertDialogCancel and
data-testid="unsaved-nav-discard" to the AlertDialogAction within the
AlertDialog that uses pendingNavDirection, setPendingNavDirection and
onNavigate; keep existing onClick handlers intact so Cancel still calls
setPendingNavDirection(null) and Discard still captures dir then calls
onNavigate?.(dir).
- Around line 215-226: The dirty-check used by handleNavigate (isDirty) omits
in-progress edits in allowedExtraHeadersRaw, so typing there and navigating
skips the confirmation; update the isDirty calculation to also consider
differences between allowedExtraHeadersRaw and the form's synced value (e.g.,
compare allowedExtraHeadersRaw !== form.getValues('allowed_extra_headers') or
!== allowedExtraHeaders if that variable exists), so handleNavigate will call
setPendingNavDirection when the raw textarea differs from the saved RHF value
before calling onNavigate.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ec09de0-107a-4026-a9a4-e8b17bebb96e

📥 Commits

Reviewing files that changed from the base of the PR and between 18253b0 and 0fc25a8.

📒 Files selected for processing (2)

• ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx
• ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx

[akshaydeo]

Merge activity

• May 27, 1:44 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 27, 2:01 PM UTC: Graphite rebased this pull request as part of a merge.
• May 27, 2:02 PM UTC: @akshaydeo merged this pull request with Graphite.

The base branch was changed.