New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(SearchBox): allow custom reset and submit components #1991

Merged
merged 2 commits into from Feb 28, 2017

Conversation

Projects
None yet
4 participants
@Haroenv
Copy link
Member

Haroenv commented Feb 17, 2017

What project are you opening a pull request for?

  • react-instantsearch (use v2 base)

Summary

Because the searchbox is icon-based, but we still want to allow people to give custom icons, the solution here is to have custom components. They start lowercase and then end with Component.

A similar solution might be used for example in pagination if this seems useful

see #1954
fixes #1952

Result

Here a text-based (span with emoji content) submit button and an svg as reset button have been chosen as demo.

screen shot 2017-02-17 at 20 46 11

feat(SearchBox): allow custom reset and submit components
Because the searchbox is icon-based, but we still want to allow people to give custom icons, the solution here is to have custom components. They start lowercase and then end with Component.

A similar solution might be used for example in pagination if this seems useful

see #1954
<svg role="img">
<use xlinkHref="#sbx-icon-search-13"></use>
</svg>
{

This comment has been minimized.

@mthuret

mthuret Feb 20, 2017

Member

I would have maybe compute this before the <form>. What do you think?

@mthuret

This comment has been minimized.

Copy link
Member

mthuret commented Feb 20, 2017

That's nice! Thanks @Haroenv

As for the stories, I'm wondering if we should considerer an example illustrating the text first way. Either as a new one or replacement?

@Haroenv

This comment has been minimized.

Copy link
Member Author

Haroenv commented Feb 20, 2017

Because of the changes in #1795 it's pretty much impossible to make the buttons have text instead of a square icon. Even with a lot of changes it's still impossible to put :active styles around the __wrapper instead of the __input. Square content works perfectly without any css change.

Here is the changes I made to use flexbox instead, which ultimately looks like this:

screen shot 2017-02-20 at 14 29 40

prototype text-based SearchBox
diff --git packages/react-instantsearch-theme-algolia/mixins/_searchbox.shipow.scss packages/react-instantsearch-theme-algolia/mixins/_searchbox.shipow.scss
index 30b844b1..ac840932 100644
--- packages/react-instantsearch-theme-algolia/mixins/_searchbox.shipow.scss
+++ packages/react-instantsearch-theme-algolia/mixins/_searchbox.shipow.scss
@@ -52,29 +52,27 @@
   &__wrapper {
     width: 100%;
     height: 100%;
+    display: flex;
+    border: 1px solid $input-border-color;
+    border-radius: even-px($border-radius);
   }
 
   &__input {
     @include input();
     display: inline-block;
     transition: box-shadow .4s ease, background .4s ease;
-    border: 1px solid $input-border-color;
-    border-radius: even-px($border-radius);
     background: $input-background;
     box-shadow: 0 1px 1px 0 rgba(85, 95, 110, 0.2);
     padding: 0;
-    padding-right: if($icon-position == 'right', even-px($input-height) + even-px($icon-clear-size) + 8px, even-px($input-height * .8))
-              + if($icon-background-opacity == 0, 0, even-px($font-size));
-    padding-left: if($icon-position == 'right',
-              even-px($font-size / 2) + even-px($border-radius / 2),
-              even-px($input-height) + if($icon-background-opacity == 0, 0, even-px($font-size * 1.2)));
     width: 100%;
     height: 100%;
     vertical-align: middle;
     white-space: normal;
     font-size: inherit;
     appearance: none;
-
+    order: 2;
+    flex-grow: 1;
+    
     &::-webkit-search-decoration,
     &::-webkit-search-cancel-button,
     &::-webkit-search-results-button,
@@ -96,26 +94,22 @@
   }
 
   &__submit {
-    position: absolute;
-    top: 0;
     @if $icon-position == 'right' {
-      right: 0;
-      left: inherit;
+      order: 3;
     } @else {
-      right: inherit;
-      left: 0;
+      order: 1;
     }
     margin: 0;
     border: 0;
     border-radius: if($icon-position == 'right', 0 $border-radius $border-radius 0, $border-radius 0 0 $border-radius);
     background-color: rgba($icon-background, $icon-background-opacity);
     padding: 0;
-    width: even-px($input-height) + if($icon-background-opacity == 0, 0, even-px($font-size / 2));
     height: 100%;
     vertical-align: middle;
     text-align: center;
     font-size: inherit;
     user-select: none;
+    flex-grow: 0;
 
     // Helper for vertical alignement of the icon
     &::before {
@@ -123,7 +117,7 @@
       margin-right: -4px;
       height: 100%;
       vertical-align: middle;
-      content: ''2
+      content: '';
     }
 
     &:hover,
@@ -145,11 +139,6 @@
 
   &__reset {
     display: none;
-    position: absolute;
-    top: (even-px($input-height) - even-px($icon-clear-size)) / 2 - 4px;
-    right: if($icon-position == 'right',
-      even-px($input-height) + if($icon-background-opacity == 0, 0 , even-px($font-size)),
-      (even-px($input-height) - even-px($icon-clear-size)) / 2 - 4px);
     margin: 0;
     border: 0;
     background: none;
@@ -158,6 +147,12 @@
     font-size: inherit;
     user-select: none;
     fill: $icon-color;
+    @if $icon-position == 'right' {
+      order: 1;
+    } @else {
+      order: 3;
+    }
+    flex-grow: 0;
 
     &:focus {
       outline: 0;
diff --git stories/SearchBox.stories.js stories/SearchBox.stories.js
index 2e764ea8..ed3289b0 100644
--- stories/SearchBox.stories.js
+++ stories/SearchBox.stories.js
@@ -8,6 +8,18 @@ const stories = storiesOf('SearchBox', module);
 
 stories.addDecorator(withKnobs);
 
+const i18n = {
+  en: {
+    search: 'search',
+    clear: 'clear',
+  }
+}
+const Intl = ({ val, lang = "en" }) => (
+  <span style={{ padding: ".2em", textTransform: "uppercase" }}>
+    {i18n[lang][val]}
+  </span>
+);
+
 stories.add('default', () =>
   <WrapWithHits searchBox={false} hasPlayground={true} linkedStoryGroup="SearchBox">
     <SearchBox/>
@@ -16,6 +28,13 @@ stories.add('default', () =>
   <WrapWithHits searchBox={false} hasPlayground={true} linkedStoryGroup="SearchBox">
     <SearchBox defaultRefinement="battery" />
   </WrapWithHits>
+).add('text based buttons', () =>
+  <WrapWithHits searchBox={false} hasPlayground={true} linkedStoryGroup="SearchBox" >
+    <SearchBox
+      submitComponent={<Intl val="search" />}
+      resetComponent={<Intl val="clear" />}
+    />
+  </WrapWithHits>
 ).add('with submit and reset components', () =>
   <WrapWithHits searchBox={false} hasPlayground={true} linkedStoryGroup="SearchBox" >
     <SearchBox

The reason this doesn't work properly is because what's the case now is that the buttons are being put "inside" the input and the input has a padding that fits the width of the buttons. This doesn't work when they can have different sizes.

This PR still works for custom icons, so I think that's valuable, because it stays within the philosophy o f #1795 of giving a good default, but if you want to have a custom searchBox, then you should write one.

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Feb 22, 2017

If I summarize this PR, we now provide a way to provide icons if they fit in the current CSS we are providing.

But if you want simple text buttons for submit you should basically have a custom searchbox? If so it means we are not fixing #1952 no?

@Haroenv

This comment has been minimized.

Copy link
Member Author

Haroenv commented Feb 22, 2017

Yes, unfortunately the way the css works makes it impossible to both have the buttons inside but text outside. This will work no problem when you have a custom css

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Feb 22, 2017

LGTM, @mthuret?

@vvo

vvo approved these changes Feb 22, 2017

@vvo vvo added this to Doing (Remove from project when done) in InstantSearch - DEPRECATED (kept for reference, to delete in may) Feb 22, 2017

@vvo vvo merged commit cd303d7 into v2 Feb 28, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@vvo vvo deleted the feat/searchbox-custom-components branch Feb 28, 2017

@vvo vvo removed this from Doing (Remove from project when done) in InstantSearch - DEPRECATED (kept for reference, to delete in may) Mar 6, 2017

Haroenv added a commit that referenced this pull request Mar 20, 2017

v3.2.2-beta0
<a name="3.2.2-beta0"></a>
## [3.2.2-beta0](v3.2.1...v3.2.2-beta0) (2017-03-20)

### Bug Fixes

* **InfiniteHits:** provide translation key for `Load More` (#2048) ([6130bf2](6130bf2))
* **SearchBox:** better mobile behaviour by default ([ea968b3](ea968b3))
* **example:** link to instantsearch/react (#2007) ([5e674cd](5e674cd))
* **recipes:** react router v4 ([de673bf](de673bf))

### Features

* **SearchBox:** add role=search to the form (#2046) ([d1e90f3](d1e90f3))
* **SearchBox:** allow custom reset and submit components (#1991) ([cd303d7](cd303d7))
* **searchBox:** add event handling ([e267ab6](e267ab6)), closes [#2017](#2017)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment