Skip to content

fix: [Bug]: Logs view keyboard navigation J/K is inverted vs UI buttons#24324

Open
RoyVivat wants to merge 1 commit intoBerriAI:mainfrom
RoyVivat:fix/issue-24279
Open

fix: [Bug]: Logs view keyboard navigation J/K is inverted vs UI buttons#24324
RoyVivat wants to merge 1 commit intoBerriAI:mainfrom
RoyVivat:fix/issue-24279

Conversation

@RoyVivat
Copy link

@RoyVivat RoyVivat commented Mar 21, 2026

fix: address issue #24279

Relevant issues

Fixes #24279

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run Link:
  • CI run for the last commit Link:
  • Merge / cherry-pick CI run Links:

Type

  • 🆕 New Feature
  • 🐛 Bug Fix
  • 🧹 Refactoring
  • 📖 Documentation
  • 🚄 Infrastructure
  • ✅ Test

@vercel
Copy link

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 21, 2026 9:18pm

Request Review

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codspeed-hq
Copy link
Contributor

codspeed-hq bot commented Mar 21, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing RoyVivat:fix/issue-24279 (55b2726) with main (2ea9e20)

Open in CodSpeed

@RoyVivat
Copy link
Author

@greptileai

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 21, 2026

Greptile Summary

This PR fixes a keyboard navigation inversion bug (#24279) in the log details drawer: previously J called selectPreviousLog and K called selectNextLog, which was the opposite of the UI button behavior. The fix swaps the two function calls in the switch statement and corrects the JSDoc comments to match. A new Vitest test file is added covering lowercase/uppercase J and K, boundary conditions (no navigation past first/last), Escape, and the closed-drawer no-op case.

Changes:

  • useKeyboardNavigation.ts: Swap selectNextLog/selectPreviousLog between the j/J and k/K cases; update JSDoc comments to reflect corrected semantics.
  • useKeyboardNavigation.test.ts (new): Comprehensive unit tests validating the corrected J → next and K → previous behavior using renderHook + synthetic keyboard events (no network calls).

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, well-tested inversion fix with no side effects.
  • The change is a single two-line swap of function calls that exactly matches the stated intent (fix inverted J/K keyboard navigation). The JSDoc was corrected in tandem, and a thorough new test suite validates every relevant case. There are no network calls, no data mutations, and no backwards-incompatible API changes.
  • No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.ts Swaps selectNextLog/selectPreviousLog between J and K key cases, and corrects the JSDoc comments accordingly — a clean, minimal inversion fix.
ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.test.ts New test file covering J/K navigation (both cases), boundary conditions, Escape, and closed-drawer behavior — all assertions align with the corrected logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[keydown event fired] --> B{isUserTyping?}
    B -- Yes --> Z[Ignore]
    B -- No --> C{isOpen?}
    C -- No --> Z
    C -- Yes --> D{e.key}
    D -- Escape --> E[onClose]
    D -- j / J --> F[selectNextLog\ncurrentIndex + 1]
    D -- k / K --> G[selectPreviousLog\ncurrentIndex - 1]
    F --> H{at last item?}
    H -- Yes --> Z
    H -- No --> I[onSelectLog allLogs currentIndex+1]
    G --> J{at first item?}
    J -- Yes --> Z
    J -- No --> K[onSelectLog allLogs currentIndex-1]
Loading

Last reviewed commit: "fix: address issue #..."

@RoyVivat RoyVivat marked this pull request as ready for review March 21, 2026 21:36
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.

[Bug]: Logs view keyboard navigation J/K is inverted vs UI buttons

2 participants