fix(components): replace reset icon on SearchInput component

[connor-baer]

Addresses #545.

Purpose

We have several problems with the SeachInput component:

• it’s kinda broken on Safari (UI for SearchInput component is broken on Safari #545)
• there is no clear icon in Firefox
• when you click on the current default clear icon, there is no handler prop provided, so you clear the input value but you can’t do anything else when this happens

Approach and changes

• Change the input type from "search" to "text"
• Add a custom button to clear the input. When clicked, it calls the new onClear handler

👀 Preview

Definition of done

• Development completed
• Reviewers assigned
• Unit and integration tests
• Meets minimum browser support
• Meets accessibility requirements

[codecov]

Codecov Report

Merging #555 into canary will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           canary    #555      +/-   ##
=========================================
+ Coverage   79.68%   79.7%   +0.01%     
=========================================
  Files         173     173              
  Lines        2491    2493       +2     
  Branches      450     451       +1     
=========================================
+ Hits         1985    1987       +2     
  Misses        389     389              
  Partials      117     117

Impacted Files	Coverage Δ
src/components/SearchInput/SearchInput.js	100% <100%> (ø)	⬆️

[vercel]

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/sumup-oss/circuit-ui/ib9hdhb0j
✅ Preview: https://circuit-ui-git-feature-add-clear-to-search-input.sumup-oss.now.sh

[connor-baer]

✅ Approved once the tests pass (don't forget to update the storyshots).

(I can't approve the PR because I opened it myself 🙄)

[connor-baer]

I've been thinking about these changes — instead of adding a new onClear handler, how about we simply call the onChange handler with an empty string?

Consider this example:

// 1. With a separate onClear prop
const Search = () => {
  const [value, setValue] = useState("");

  const handleChange = event => {
    setValue(event.target.value);
  };

  const handleClear = () => {
    setValue('');
  }

  return <SearchInput value={value} onChange={handleChange} onClear={handleClear} />;
};

// 2. The clear button just calls `onChange('')`
const Search = () => {
  const [value, setValue] = useState('');

  const handleChange = (event) => {
    setValue(event.target.value);
  }

  return (
    <SearchInput value={value} onChange={handleChange} />
  );
}

The 2. example is more concise and more intuitive since the developer doesn't need to implement an additional handler. The clearing functionality would come "out of the box".

Thank you for considering these thoughts so late in the game. I think it's important that we get the component APIs in Circuit UI right.

[robinmetral]

👏

[marielakas]

I've been thinking about these changes — instead of adding a new onClear handler, how about we simply call the onChange handler with an empty string?

Consider this example:

// 1. With a separate onClear prop
const Search = () => {
  const [value, setValue] = useState("");

  const handleChange = event => {
    setValue(event.target.value);
  };

  const handleClear = () => {
    setValue('');
  }

  return <SearchInput value={value} onChange={handleChange} onClear={handleClear} />;
};

// 2. The clear button just calls `onChange('')`
const Search = () => {
  const [value, setValue] = useState('');

  const handleChange = (event) => {
    setValue(event.target.value);
  }

  return (
    <SearchInput value={value} onChange={handleChange} />
  );
}

The 2. example is more concise and more intuitive since the developer doesn't need to implement an additional handler. The clearing functionality would come "out of the box".

Thank you for considering these thoughts so late in the game. I think it's important that we get the component APIs in Circuit UI right.

As discussed in Slack, I'm going to keep the current implementation. ;)

[ghost]

🎉 This PR is included in version 1.7.1-canary.2 🎉

The release is available on:

• npm package (@canary dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ghost]

🎉 This PR is included in version 1.8.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 2.0.0-alpha.10 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀