Skip to content

Conversation

@gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Nov 13, 2025

Ticket ENG-1979

Description Of Changes

Simplified the component by:

  • Removing the separate Clear button in favor of Ant's built-in allowClear
  • Removing the Space.Compact wrapper
  • Setting withIcon=true as the default
  • take advantage of Ant's built in onClear

Code Changes

  • Replaced Chakra SearchLineIcon with Carbon Icons.Search
  • Converted from Space.Compact wrapper to single Input component
  • Removed separate Clear button
  • Changed withIcon to default to true
  • Updated placeholder from "Search..." to "Search"
  • Added aria-label="Search" for accessibility
  • Removed withIcon prop from two call sites where it's now the default
  • Added CSS variable override for icon spacing

Steps to Confirm

  1. Navigate to any page with a search input (e.g., Systems, User Management, Data Discovery)
  2. Verify the search icon appears and is properly styled
  3. Type in the search input and verify filtering works
  4. Click the clear button (X) and verify it clears the input

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link

vercel bot commented Nov 13, 2025

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

Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ready Ready Preview Comment Nov 14, 2025 9:25pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Nov 14, 2025 9:25pm

@gilluminate gilluminate marked this pull request as ready for review November 14, 2025 00:01
@gilluminate gilluminate requested a review from a team as a code owner November 14, 2025 00:01
@gilluminate gilluminate requested review from speaker-ender and removed request for a team November 14, 2025 00:01
@greptile-apps

This comment was marked as outdated.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gilluminate
Copy link
Contributor Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. clients/admin-ui/src/features/common/DebouncedSearchInput.tsx, line 41 (link)

    syntax: onClear prop removed from SearchInput but still referenced here - causes type error

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@jack-gale-ethyca jack-gale-ethyca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic

@gilluminate gilluminate added this pull request to the merge queue Nov 14, 2025
Merged via the queue into main with commit 23a6c80 Nov 14, 2025
47 checks passed
@gilluminate gilluminate deleted the gill/ENG-1979/update-search-input branch November 14, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants