Skip to content

feat: add clear button in search input#1895

Merged
ghostdevv merged 5 commits intonpmx-dev:mainfrom
SahulKola:fix/add-clear-icon-in-search-field-for-mobile-view
Mar 5, 2026
Merged

feat: add clear button in search input#1895
ghostdevv merged 5 commits intonpmx-dev:mainfrom
SahulKola:fix/add-clear-icon-in-search-field-for-mobile-view

Conversation

@SahulKola
Copy link
Contributor

@SahulKola SahulKola commented Mar 3, 2026 •
edited by ghostdevv
Loading

🔗 Linked issue

none

🧭 Context

📚 Description

image

@vercel
Copy link

vercel bot commented Mar 3, 2026 •
edited
Loading

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 5, 2026 4:04am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 5, 2026 4:04am
npmx-lunaria Ignored Ignored Mar 5, 2026 4:04am

Request Review

@SahulKola SahulKola force-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from f619f50 to 2b9581d Compare March 3, 2026 15:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: acfcf690-94a2-4149-a38a-1dc7ed3a121d

📥 Commits

Reviewing files that changed from the base of the PR and between 8bcbeed and fd2a652.

📒 Files selected for processing (1)
  • app/components/Header/SearchBox.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Header/SearchBox.vue
📝 Walkthrough

Walkthrough

The Header SearchBox component adds a computed hasSearchQuery to detect a non-empty searchQuery, and a clearSearch method that resets searchQuery and focuses the input. A clear button (circle-x icon with an aria-label) is conditionally rendered via v-if="hasSearchQuery" and triggers clearSearch on click. InputBase styling receives additional end padding (pe-8) to accommodate the button. Focus exposure and existing focus behaviour remain unchanged.

Suggested reviewers

  • danielroe
  • alexdln
  • whitep4nth3r
🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description contains only scaffolded template sections with an image and no substantive text explaining the changes, context, or implementation details. Add a detailed description explaining what the clear button feature does, why it was implemented, and how it improves the user experience, particularly for mobile users.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Header/SearchBox.vue 25.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

coderabbitai[bot]
coderabbitai bot reviewed Mar 3, 2026
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d70dd8 and 2b9581d.

📒 Files selected for processing (1)
  • app/components/Header/SearchBox.vue

@danielroe danielroe changed the title fix: add clear button in search field (mobile view) - #1881 fix: add clear button in search field (mobile view) Mar 3, 2026
@SahulKola SahulKola force-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from 2b9581d to 747a1dd Compare March 3, 2026 16:11
@SahulKola SahulKola force-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from 747a1dd to 8bcbeed Compare March 3, 2026 19:23
@ghostdevv ghostdevv added the front Frontend, Design label Mar 5, 2026
@ghostdevv ghostdevv changed the title fix: add clear button in search field (mobile view) feat: add clear button in search input Mar 5, 2026
@ghostdevv
Copy link
Contributor

ghostdevv commented Mar 5, 2026 •
edited
Loading

thanks for the PR! will share in the Discord (message) to get some feedback 👀

It doesn't seem however to be a fix for #1881 which asked for a way to hide the close the search box on mobile. When you open the site if there is no text in the search box it's closed (hidden) on mobile by default. I updated your description and pr title to fix that

knowler
knowler reviewed Mar 5, 2026
View reviewed changes
@ghostdevv ghostdevv enabled auto-merge March 5, 2026 04:05
@ghostdevv ghostdevv added this pull request to the merge queue Mar 5, 2026
Merged via the queue into npmx-dev:main with commit e42fff4 Mar 5, 2026
20 checks passed
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

Thanks for your first contribution, @SahulKola! 🎊

We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role.

@github-actions github-actions bot mentioned this pull request Mar 5, 2026
Merged
@alex-key
Copy link
Contributor

alex-key commented Mar 5, 2026

@SahulKola Is there any particular reason clear button is not focusable?

aria-hidden="true"
tabindex="-1"

@ghostdevv
Copy link
Contributor

ghostdevv commented Mar 5, 2026

@SahulKola Is there any particular reason clear button is not focusable?

aria-hidden="true"
tabindex="-1"

@alex-key
Copy link
Contributor

alex-key commented Mar 5, 2026

@SahulKola Is there any particular reason clear button is not focusable?

aria-hidden="true"
tabindex="-1"

Thanks, I missed that

@ghostdevv
Copy link
Contributor

ghostdevv commented Mar 5, 2026

np!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@knowler knowler knowler left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@ghostdevv ghostdevv ghostdevv approved these changes

Assignees

No one assigned

Labels

front Frontend, Design

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SahulKola @ghostdevv @alex-key @knowler