Skip to content

feat: add clear button in search input - #1895

Merged
ghostdevv merged 5 commits into
npmx-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 into
npmx-dev:mainfrom
SahulKola:fix/add-clear-icon-in-search-field-for-mobile-view

Conversation

@SahulKola

@SahulKolaSahulKola commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

🔗 Linked issue

none

🧭 Context

📚 Description

image

@vercel

vercelBot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

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

ProjectDeploymentActionsUpdated (UTC)
npmx.devReadyReadyPreview, CommentMar 5, 2026 4:04am
2 Skipped Deployments
ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devIgnoredIgnoredPreviewMar 5, 2026 4:04am
npmx-lunariaIgnoredIgnoredMar 5, 2026 4:04am

Request Review

@SahulKola
SahulKolaforce-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from f619f50 to 2b9581dCompareMarch 3, 2026 15:45
@coderabbitai

coderabbitaiBot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

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 nameStatusExplanationResolution
Description check❓ InconclusiveThe 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

codecovBot commented Mar 3, 2026

Copy link
Copy Markdown

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 linesPatch %Lines
app/components/Header/SearchBox.vue25.00%3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

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

Comment threadapp/components/Header/SearchBox.vue
@danielroedanielroe changed the title fix: add clear button in search field (mobile view) - #1881fix: add clear button in search field (mobile view)Mar 3, 2026
@SahulKola
SahulKolaforce-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from 2b9581d to 747a1ddCompareMarch 3, 2026 16:11
@SahulKola
SahulKolaforce-pushed the fix/add-clear-icon-in-search-field-for-mobile-view branch from 747a1dd to 8bcbeedCompareMarch 3, 2026 19:23
@ghostdevvghostdevv added the front Frontend, Design label Mar 5, 2026
@ghostdevvghostdevv changed the title fix: add clear button in search field (mobile view)feat: add clear button in search inputMar 5, 2026
@ghostdevv

ghostdevv commented Mar 5, 2026

Copy link
Copy Markdown
Member

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

Comment threadapp/components/Header/SearchBox.vue Outdated
@ghostdevv
ghostdevv enabled auto-merge March 5, 2026 04:05
@ghostdevv
ghostdevv added this pull request to the merge queueMar 5, 2026
Merged via the queue into npmx-dev:main with commit e42fff4Mar 5, 2026
20 checks passed
@github-actions

Copy link
Copy Markdown

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-actionsgithub-actionsBot mentioned this pull request Mar 5, 2026
@alex-key

Copy link
Copy Markdown
Contributor

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

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

@ghostdevv

Copy link
Copy Markdown
Member

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

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

@alex-key

Copy link
Copy Markdown
Contributor

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

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

Thanks, I missed that

@ghostdevv

Copy link
Copy Markdown
Member

np!

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

Labels

frontFrontend, Design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@SahulKola@ghostdevv@alex-key@knowler