fix: fix upward arrow navigation in virtual scrolling#60
Conversation
Fixed a bug where upward arrow key navigation would incorrectly jump between categories instead of treating category headers as regular list elements. Refactored navigation logic using ts-pattern for better code maintainability and consistency between up/down arrow behaviors.
|
""" WalkthroughThe refactor extracts filtering, flattening, and navigation logic from the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learningssrc/sub-agent-scanner.ts (1)Learnt from: CR 🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @nyatinte, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've addressed a bug in the virtual scrolling implementation where upward arrow key navigation would incorrectly jump between categories. This PR ensures a smoother user experience by correctly handling transitions between files and group headers, and I've also refactored the navigation logic for better maintainability and consistency.
Highlights
- Fixed Upward Arrow Navigation: The upward arrow navigation in the virtual scrolling component now correctly moves from files to their group headers before transitioning to previous groups, ensuring a more intuitive user experience.
- Refactored Navigation Logic: The existing navigation logic has been refactored to utilize the "ts-pattern" library, replacing complex nested if-else chains with a more declarative and maintainable pattern matching approach.
- Unified Navigation Behavior: The behavior for both up and down arrow key navigation has been unified, providing consistent movement within the file list.
- Removed Unused Helper Functions: Several helper functions that became redundant after the navigation logic refactoring have been removed, cleaning up the codebase.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
commit: |
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, successfully fixing the upward arrow key navigation in the virtual scrolling list. The refactoring of the navigation logic using ts-pattern has significantly enhanced code readability and maintainability by replacing complex conditional chains with a declarative pattern matching approach. My review includes one suggestion to further simplify the new handleUpArrow function by removing a redundant check, which is possible thanks to the guarantees provided by the pattern matching.
- Create navigation-utils.ts with pure functions for file list navigation - Extract filterFileGroups for search filtering logic - Extract flattenFileGroups for hierarchy flattening - Extract navigation position calculation and arrow key handling - Add comprehensive InSource tests for all extracted functions - Improve code maintainability and testability
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/FileList/FileList.tsx(4 hunks)src/components/FileList/navigation-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/FileList/FileList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: All TypeScript code must use 'type' instead of 'interface' for type definitions.
Regular (non-component) functions must be defined using arrow function syntax.
Avoid default exports except for page components.
Never use the 'any' type; it is strictly forbidden.
Avoid 'as' type assertions; use proper type guards instead.
Remove unused imports and exports immediately to keep dependencies clean.
All TypeScript code must pass with zero type errors (strict mode enforced).
All code must pass Biome lint/format checks with zero errors.
All code must have zero unused dependencies, exports, or types (enforced by Knip).
All code must use immutable design with 'readonly' properties throughout TypeScript code.
All optional properties in TypeScript must use 'exactOptionalPropertyTypes: true' (no '| undefined' on optional props).
Array access in TypeScript must account for 'noUncheckedIndexedAccess: true' (array access returns 'T | undefined').
All code paths in TypeScript functions must return a value ('noImplicitReturns: true').
Files:
src/components/FileList/navigation-utils.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Avoid default exports except for page components.
src/components/FileList/navigation-utils.ts (1)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
🧬 Code Graph Analysis (1)
src/components/FileList/navigation-utils.ts (1)
src/_types.ts (2)
FlatItem(75-81)ClaudeFileInfo(28-35)
🔇 Additional comments (7)
src/components/FileList/navigation-utils.ts (7)
7-25: Well-implemented filtering logic with proper immutabilityThe function correctly handles immutability by creating new arrays and objects, and the case-insensitive search on both filename and full path provides good user experience.
30-41: Efficient flattening implementationGood use of
flatMapfor performance and proper handling of expanded/collapsed states.
46-62: Well-structured navigation position typeComprehensive type definition with proper readonly properties and appropriate nullable handling.
106-110: Clean result type definitionSimple and effective type with proper readonly properties.
115-166: Comprehensive upward navigation handlingThe implementation correctly handles all navigation cases, including the fix for moving from first file to group header as mentioned in the PR objectives.
242-541: Excellent test coverageComprehensive in-source tests following the required pattern, with good coverage of edge cases and proper test data setup.
237-239: Ensure safe array accessAdd explicit null check for array access:
- const group = filteredGroups[currentGroupIndex]; - if (!group || !group.isExpanded || group.files.length === 0) return null; + const group = filteredGroups[currentGroupIndex] ?? null; + if (group == null || !group.isExpanded || group.files.length === 0) return null;⛔ Skipped due to learnings
Learnt from: CR PR: nyatinte/ccexp#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-27T15:51:39.320Z Learning: Applies to **/*.{ts,tsx} : Array access in TypeScript must account for 'noUncheckedIndexedAccess: true' (array access returns 'T | undefined').
…mments - Cache lowercase search query to avoid repeated conversions - Remove verbose JSDoc comments that were redundant with function names - Follow project's comment guidelines (only keep 'why' comments)
…ssary comments - Use es-toolkit/compat's filter function for better performance - Cache file path toLowerCase to avoid repeated conversions - Remove redundant inline comments that were self-explanatory - Follow project's comment guidelines (only keep 'why' comments)
Set consistent terminal height (24 rows) in the test to ensure predictable viewport calculations across different environments. The test was failing in CI due to different terminal dimensions affecting virtual scroll behavior.
- Add nullish coalescing for array access in calculateNavigationPosition - Fix potential undefined array access in handleDownArrowNavigation - Update getFileAtPosition with proper null checks - Ensures compliance with noUncheckedIndexedAccess TypeScript setting
- Filter test results by name and scope to avoid conflicts with user agents - Change to recursive scanning to ensure nested test files are found - Tests now properly isolate project-scoped agents from user-level ones
1 participant
Overview
This PR fixes a bug in the virtual scrolling implementation where upward arrow key navigation would incorrectly jump between categories instead of treating category headers as regular list elements.
Changes
ts-patternfor better code maintainabilityImplementation
The key fix involves ensuring that when navigating upward from the first file in a group, the selection moves to the group header first (same behavior as downward navigation) instead of jumping directly to the previous group.
Used
ts-patternto replace complex nested if-else chains with declarative pattern matching:Related
Summary by CodeRabbit