Skip to content
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

How to pass default SearchParameters #1342

Closed
vvo opened this issue Sep 26, 2016 · 18 comments
Closed

How to pass default SearchParameters #1342

vvo opened this issue Sep 26, 2016 · 18 comments

Comments

@vvo
Copy link
Contributor

vvo commented Sep 26, 2016

With our focus on widget states, we missed one important usecase: being able to define simple SearchParameters that will always be sent.

This goes back to the idea of @bobylito of having a <Configure/> widget. For instance we could imagine having such simple <Configure/> widget that we pass a state to search parameters function that will allow users to set distinct=true for instance or any other Search parameter.

What do you think?

@vvo vvo added this to the react-instantsearch first release milestone Sep 26, 2016
@harshmaur
Copy link

Really need this.

In my own index, I have by default set distinct = true

But in my UI I have use cases where I want to set it to false.

@vvo
Copy link
Contributor Author

vvo commented Oct 9, 2016

This is planned and need to be done, yes

@harshmaur
Copy link

harshmaur commented Oct 9, 2016

Hi @vvo I was trying to find how to do it with current beta 13. Any leads?

@vvo
Copy link
Contributor Author

vvo commented Oct 9, 2016

I would suggest you to clone the projet, create a new "Connector" see how the searchBox connector is done, then see how a generic "configure" component could be done. Should be doable, but you will have to dig in the docs (making your own widget: https://community.algolia.com/instantsearch.js/react/Customization.html) and the code. Good luck.

@harshmaur
Copy link

harshmaur commented Oct 9, 2016

Got it, I was looking for this page only. It wasnt linked in the current new docs.

@harshmaur
Copy link

harshmaur commented Oct 9, 2016

Thanks I used the hits connecter from docs https://github.com/algolia/instantsearch.js/blob/v2/packages/react-instantsearch/src/widgets/Hits/connect.js

My getSearchParameters looks like this now.

getSearchParameters(searchParameters, props) {
    // set distinct to false. 
    searchParameters = searchParameters.setQueryParameter("distinct", false);
    if (
        typeof props.hitsPerPage !== 'undefined' &&
        typeof searchParameters.hitsPerPage === 'undefined'
    ) {
      return searchParameters.setHitsPerPage(props.hitsPerPage);
    }
    return searchParameters;
  }

@vvo
Copy link
Contributor Author

vvo commented Oct 11, 2016

Awesome! Utilmately we need a simpler API to do that.

@bobylito bobylito self-assigned this Oct 21, 2016
@bobylito
Copy link
Contributor

bobylito commented Oct 21, 2016

What about a function passed as a prop on InstantSearch with the signature SearchParameters => SearchParameters We don't really need to provide the SearchParameter object but I think it's easier than having to instantiate it yourself (as a developer)

@vvo
Copy link
Contributor Author

vvo commented Oct 21, 2016

We don't really need to provide it but I think it's easier than having to instantiate it yourself (as a developer)

Not sure to get what you are proposing

@bobylito
Copy link
Contributor

Updated my comment @vvo

@vvo
Copy link
Contributor Author

vvo commented Oct 21, 2016

We don't really need to provide the SearchParameter object but I think it's easier than having to instantiate it yourself (as a developer)

Yes it is easier to manipulate an existing state rather than having to provide an API to create a state.

As for an option on <InstantSearch/> or a <Configure/> widget, with a <Configure/> widget, you could maybe more easily remove refinements dynamically if you unmount the component. Example:

var WithGeo = <Configure state={state => state.addQueryParameters('geo', '...')} />
var WithDistinct = <Configure state={state => state.addQueryParameters('geo', '...')} />
// later on..
<WithGeo/><WithDistinct/>

// and remove them when needed.

I guess the same is achievable with just a function on <InstantSearch/>?

@vvo vvo mentioned this issue Oct 21, 2016
10 tasks
@bobylito
Copy link
Contributor

That's an interesting declarative Api using configure. What do you think @mthuret?

@mthuret
Copy link
Contributor

mthuret commented Oct 25, 2016

Having a function that configure SearchParameter on the component feel more natural to me than having several widget. But if it's really easier for changing refinements dynamically that's a real advantage.

In either case that's great, I will need this feature in a near future :)

@vvo
Copy link
Contributor Author

vvo commented Oct 25, 2016

having several widget

Sorry the example was misleading, we won't have several widget, only one <configure/> widget.

The issue I see if we put it as an option on <InstantSearch/> is about being able to unmount/remove parameters. I am not sure how that would fit.

This is a good investigation issue.

@bobylito
Copy link
Contributor

I've given some thoughts 🤔 on the <Configure/> vs <InstantSearch /> discussion 💡

The issue I see if we put it as an option on is about being able to unmount/remove parameters. I am not sure how that would fit.

The function on <InstantSearch /> will be a prop. therefore it can be easily switched.

var WithGeo = state => state.addQueryParameter('geo', '...')};
var WithDistinct = state => state.addQueryParameter('distinct', '...')};
// later on..
<InstantSearch searchStateConfig={withGeo}/>
// Or
<InstantSearch searchStateConfig={withGeo}/>
// And it can be composed too
<InstantSearch searchStateConfig={s => withDistinct(withGeo(s)) }/>

Also:

  • it'll be easier to predict when the function is applied if we control it from this component.
  • the API will be more familiar and does not rely on non-UI widget widget

I suggest we go for a prop on <InstantSearch /> 🚀

@vvo
Copy link
Contributor Author

vvo commented Oct 25, 2016

WFM, I think I was up and proposed for the <Configure/> widget mostly because it is inlined with the <VirtualWidget/> usage (widget without any rendering, only for state). But let's move on as your proposed

@vvo
Copy link
Contributor Author

vvo commented Oct 25, 2016

(Also don't try to millennial-convince us lowl)

@bobylito
Copy link
Contributor

it always works better to convey the messsage 😸

vvo pushed a commit that referenced this issue Oct 26, 2016
#1471)

* core(instantsearch-manager): provide a fn for setting SearchParameters

FIX #1342

* core(InstantSearch): accept an optional setting object to send with the request
@vvo vvo closed this as completed Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants