-
Notifications
You must be signed in to change notification settings - Fork 21
Fix: Improve CoreBox shrink behavior when no recommendation #227
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
base: master
Are you sure you want to change the base?
Conversation
- Fix recommendation timeout handling to explicitly trigger collapse - Improve resetSearchState to trigger collapse when input is empty - Force minimum height when no results and not waiting for data - Keep collapsed state when waiting for data if already collapsed This fix addresses the issue where CoreBox would not shrink when: 1. Recommendation is disabled or times out 2. Input is cleared but window stays expanded 3. No results are available but window doesn't collapse The changes ensure CoreBox properly shrinks to minimum height when there are no results, no input, and no pending operations.
📝 WalkthroughWalkthroughThese changes introduce collapse/expand behavior to a search box component when results are empty. The main process validates collapsed state preservation, the resize hook conditionally applies minimum height, and the search hook triggers collapse on input reset and recommendation timeout. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/core-app/src/renderer/src/modules/box/adapter/hooks/useResize.ts`:
- Around line 76-82: The brace-style lint error is caused by the `else` being on
the same line as the closing brace in the conditional that sets `height` (using
`resultCount`, `isLoading`, `isRecommendationPending`, `MIN_HEIGHT`, and
`calculateDesiredHeight`); fix it by moving `else` to its own line and
formatting the if/else block to match the project's brace-style (i.e., place the
closing brace on its own line followed by a newline then `else`), ensuring the
`height` assignment logic remains the same in `useResize.ts`.
🧹 Nitpick comments (1)
apps/core-app/src/renderer/src/modules/box/adapter/hooks/useSearch.ts (1)
364-370: Avoid duplicate collapse dispatch on timeout.
resetSearchState()now collapses when input is empty, so the extraCoreBoxEvents.ui.expandsend here triggers redundant IPC. Consider removing or gating it.♻️ Proposed simplification
- // Explicitly trigger collapse when recommendation times out - transport.send(CoreBoxEvents.ui.expand, { mode: 'collapse' }).catch(() => {})
| // When no results and not waiting for data, force minimum height | ||
| let height: number | ||
| if (resultCount === 0 && !isLoading && !isRecommendationPending) { | ||
| height = MIN_HEIGHT | ||
| } else { | ||
| height = calculateDesiredHeight(resultCount) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix ESLint brace-style violation.
ESLint reports style/brace-style at Line 80 because else is on the same line as the closing brace. Adjust to the configured style to avoid lint failures.
🧹 Suggested fix
- } else {
+ }
+ else {
height = calculateDesiredHeight(resultCount)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // When no results and not waiting for data, force minimum height | |
| let height: number | |
| if (resultCount === 0 && !isLoading && !isRecommendationPending) { | |
| height = MIN_HEIGHT | |
| } else { | |
| height = calculateDesiredHeight(resultCount) | |
| } | |
| // When no results and not waiting for data, force minimum height | |
| let height: number | |
| if (resultCount === 0 && !isLoading && !isRecommendationPending) { | |
| height = MIN_HEIGHT | |
| } | |
| else { | |
| height = calculateDesiredHeight(resultCount) | |
| } |
🧰 Tools
🪛 ESLint
[error] 80-80: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🤖 Prompt for AI Agents
In `@apps/core-app/src/renderer/src/modules/box/adapter/hooks/useResize.ts` around
lines 76 - 82, The brace-style lint error is caused by the `else` being on the
same line as the closing brace in the conditional that sets `height` (using
`resultCount`, `isLoading`, `isRecommendationPending`, `MIN_HEIGHT`, and
`calculateDesiredHeight`); fix it by moving `else` to its own line and
formatting the if/else block to match the project's brace-style (i.e., place the
closing brace on its own line followed by a newline then `else`), ensuring the
`height` assignment logic remains the same in `useResize.ts`.
Problem
CoreBox does not properly shrink to minimum height when:
This creates a poor user experience where the window remains unnecessarily large.
Root Cause Analysis
Issue 1: Recommendation Timeout Not Triggering Collapse
Location:
useSearch.ts- recommendation timeout handlerThe timeout handler would call
resetSearchState()but not explicitly trigger a collapse, leaving the window in an expanded state.Issue 2: resetSearchState Not Triggering Collapse
Location:
useSearch.ts-resetSearchState()functionWhen resetting search state, the function would dispatch a layout refresh event but not explicitly trigger collapse when input is empty.
Issue 3: Height Calculation Not Forcing Minimum
Location:
useResize.ts-sendLayoutUpdate()functionThe height calculation would use
calculateDesiredHeight()even when there are no results and no pending operations, instead of forcing minimum height.Issue 4: Waiting State Not Preserving Collapsed State
Location:
core-box/index.ts-applyLayoutUpdate()functionWhen waiting for data (loading/recommendation), the function would keep current size but not check if already collapsed, potentially expanding unnecessarily.
Solution
Fix 1: Explicit Collapse on Recommendation Timeout
Fix 2: Trigger Collapse in resetSearchState
Fix 3: Force Minimum Height When Appropriate
Fix 4: Preserve Collapsed State When Waiting
Changes
Modified Files
useSearch.ts
resetSearchState()when input is emptyuseResize.ts
core-box/index.ts
Testing
The fix should be tested with:
Impact
Related Issues
This PR addresses the CoreBox shrink issue reported by users.
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.