feat(search): add responsive search with manual expansion#2336
feat(search): add responsive search with manual expansion#2336
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds responsive header layout: parent header tracks available right-side space (ResizeObserver), manages manual search expansion and scroll state, and passes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/index.tsx (1)
253-262: Consider simplifying the conditional logic.The double negative
!(isSearchManuallyExpanded && !hasSpaceForSearch)is logically correct but harder to parse.Apply this diff for better readability:
- {!(isSearchManuallyExpanded && !hasSpaceForSearch) && ( + {(!isSearchManuallyExpanded || hasSpaceForSearch) && ( <Button onClick={handleNewEmptyTab} variant="ghost" size="icon" className="text-neutral-600" > <PlusIcon size={16} /> </Button> )}Both expressions are logically equivalent (De Morgan's law), but the refactored version more directly expresses "show when NOT manually expanded OR when there IS space."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/index.tsx(3 hunks)apps/desktop/src/components/main/body/search.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/search.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Never do manual state management for form/mutation. UseuseFormfrom tanstack-form anduseQuery/useMutationfrom tanstack-query for 99% cases.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/search.tsx
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/search.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/search.tsx
🧬 Code graph analysis (2)
apps/desktop/src/components/main/body/index.tsx (2)
apps/desktop/src/components/main/body/update.tsx (1)
Update(11-79)apps/desktop/src/components/main/body/search.tsx (1)
Search(11-67)
apps/desktop/src/components/main/body/search.tsx (2)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(261-267)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (8)
apps/desktop/src/components/main/body/index.tsx (4)
106-109: LGTM!State declarations are appropriate for tracking manual expansion and available space.
111-126: Verify the 220px threshold is appropriate for your design.The ResizeObserver implementation is correct with proper cleanup. However, the 220px threshold appears to be a fixed value that determines when the search bar switches between collapsed and expanded states.
Ensure this threshold value:
- Accounts for the minimum width needed to display the expanded search comfortably (currently 180px based on search.tsx line 109)
- Leaves adequate buffer space for other header elements
- Aligns with your responsive design breakpoints
You can test this by resizing the window and observing the transition point.
169-169: LGTM!Adding
shrink-0prevents the toggle button from shrinking and maintains layout stability.
264-270: LGTM!The integration with the Search component is correct:
hasSpaceprop communicates available space to enable responsive behavioronManualExpandChangecallback tracks manual expansion stateml-autoproperly positions the search controls to the rightapps/desktop/src/components/main/body/search.tsx (4)
11-17: LGTM!The props are properly typed inline per coding guidelines. The signature clearly defines:
hasSpace: required boolean for space-aware renderingonManualExpandChange: optional callback for parent coordination
23-25: LGTM with a note on callback stability.The effect correctly propagates manual expansion state to the parent. Since
setIsSearchManuallyExpandedfrom the parent (line 268 in index.tsx) is a stable setState function, the dependency array is safe and won't cause unnecessary re-renders.
105-109: Verify the width behavior change is intentional.When
!hasSpace(manually expanded on narrow screens), the search bar now uses a fixedw-[180px]regardless of focus state, whereas whenhasSpaceis true, it expands from 180px to 250px on focus.This means manually expanded search on narrow screens won't grow wider when focused. Confirm this matches your intended UX:
- Before: Unknown (removed code not shown)
- After: Manual expansion always 180px; auto-expanded can grow to 250px on focus
Test by:
- Narrowing window until search collapses
- Clicking to manually expand
- Focusing the input and verifying 180px width is adequate for user input
90-96: LGTM!The ExpandedSearch signature correctly accepts
hasSpacefor width calculation, along with focus/blur handlers for manual expansion control.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/package.json (1)
116-116: Clarify the purpose for addingusehooks-tsif overlapping with@uidotdev/usehooks.With both
usehooks-tsand@uidotdev/usehooks(line 83) now in dependencies, verify that this addition serves a specific need not met by the existing library. Both libraries offer similar hooks likeuseLocalStorageanduseCopyToClipboard. Ifusehooks-tsis being added purely for additional functionality not present in@uidotdev/usehooks, document this decision. If there is significant overlap, consider consolidating to a single library.Note:
usehooks-ts@3.1.1is a stable, actively maintained release with no known security vulnerabilities. The library is tree-shakable, so unused hooks will not increase bundle size.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/desktop/package.json(1 hunks)apps/desktop/src/components/main/body/index.tsx(5 hunks)eslint.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/main/body/index.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
eslint.config.jsapps/desktop/package.json
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
eslint.config.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
eslint.config.js
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:24.348Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/desktop-e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:24.348Z
Learning: Applies to apps/desktop-e2e/**/*.{test,spec}.{js,ts,tsx} : Refer to `scripts/setup-desktop-e2e.sh` for end-to-end test environment setup if setup is missing
Applied to files:
eslint.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
🔇 Additional comments (1)
eslint.config.js (1)
7-7: LGTM!The ignore pattern for generated files is appropriate and follows common conventions.
2 participants
Description