fix: hide mobile search field on blur#2852
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughRefactors mobile search focus/blur handling in SearchBox.vue (adds handleFocus/handleBlur, emits focus/blur, exposes blur) and updates mobile header ButtonBase class lists in AppHeader.vue to add a py-2.5! padding override for consistent vertical alignment. ChangesMobile search input handling and styling
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/AppHeader.vue (1)
328-334:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBind search button
aria-expandedto search state, not menu state.The mobile search trigger currently uses
showMobileMenu, so assistive tech gets the wrong expanded/collapsed state for this control. UseisSearchExpandedhere.- :aria-expanded="showMobileMenu" + :aria-expanded="isSearchExpanded"🤖 Prompt for 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. In `@app/components/AppHeader.vue` around lines 328 - 334, The ButtonBase mobile search trigger binds aria-expanded to the wrong state; change the aria-expanded binding from showMobileMenu to isSearchExpanded so assistive tech reflects the search panel's state. Update the ButtonBase component instance (the element using `@click`="expandMobileSearch" and v-if="!isSearchExpanded && !isOnHomePage") to use :aria-expanded="isSearchExpanded" and ensure any related logic in expandMobileSearch and the isSearchExpanded reactive/computed property remains consistent.
🤖 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 `@app/components/Header/SearchBox.vue`:
- Around line 33-43: The current handleFocus/handleBlur are used both as DOM
event handlers and as exposed imperative methods which causes duplicate emits
when calling searchBoxRef.value?.focus(); split responsibilities: keep the DOM
handlers (rename to onInputFocus/onInputBlur or similar) to set isSearchFocused
and emit('focus'/'blur'), and define separate exposed methods (focus and blur in
defineExpose) that only call inputRef.value?.focus()/blur() and set
isSearchFocused without calling emit; update defineExpose to expose those new
focus/blur methods and update any template/event binding to use the DOM handlers
(onInputFocus/onInputBlur).
---
Outside diff comments:
In `@app/components/AppHeader.vue`:
- Around line 328-334: The ButtonBase mobile search trigger binds aria-expanded
to the wrong state; change the aria-expanded binding from showMobileMenu to
isSearchExpanded so assistive tech reflects the search panel's state. Update the
ButtonBase component instance (the element using `@click`="expandMobileSearch" and
v-if="!isSearchExpanded && !isOnHomePage") to use
:aria-expanded="isSearchExpanded" and ensure any related logic in
expandMobileSearch and the isSearchExpanded reactive/computed property remains
consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3d27dc2-88d8-434b-95b5-66b1eb951006
📒 Files selected for processing (2)
app/components/AppHeader.vueapp/components/Header/SearchBox.vue
🔗 Linked issue
Resolves #1881, Closes #2021
🧭 Context
Hiding the search field in the header on mobile devices when blurred (unfocused). We had logic for this, but the field component itself didn't support it, so I simply added correct events emit
Also adjusted the button sizes a bit so that they are in a better proportion to each other