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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Oct 26, 2016
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebaebb0
core(instantsearch-manager): provide a fn for setting SearchParameters
Oct 25, 2016
0da67a6
Merge branch 'v2' into v2-configuration
bobylito Oct 25, 2016
2c59f0d
core(InstantSearch): accept an optional setting object to send with t…
Oct 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/react-instantsearch/src/core/InstantSearch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ function validateNextProps(props, nextProps) {
* @propType {string} appId - The Algolia application id.
* @propType {string} apiKey - Your Algolia Search-Only API key.
* @propType {string} indexName - The index in which to search.
* @propType {function=identity} configureSearchParameters - function to tweak the parameters sent to Algolia.
Copy link
Contributor

Choose a reason for hiding this comment

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

What we want to convey is that by default there's nothing being done. By saying the default value is "identity", any user not knowing what identity functions are (like I was at some point) may be confused.

Should we make sure everyone understands what "identity" means here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a pretty good question, I have no idea if this concept is well known or if we should make it more approachable. The nice thing of using identity is that it convey perfectly what it means, there is no approximation about its meaning, on the contrary of using words. On the other hand, if you don't know this vocab, you don't know it...

What do you think @mthuret? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some information about the behaviour of identity. Hope it's enough :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely we should explain what identity means because I'm sure some people will not know, and that can be frustrating. Your explanation is fine, and associated with an example (on the guide you mention @vvo ?), it would be imho nice :)

* It is executed after the [SearchParameters](https://community.algolia.com/algoliasearch-helper-js/reference.html#searchparameters)
* are resolved from the widgets. It can contain some logic to conditionally apply some parameters based on the state.
* Signature `SearchParameters => SearchParameters`. By default its value is `identity`, a function
* that takes one argument and returns it unmodified.
* @propType {bool=true} urlSync - Enables automatic synchronization of widgets state to the URL. See [URL Synchronization](#url-synchronization).
* @propType {object} history - A custom [history](https://github.com/ReactTraining/history) to push location to. Useful for quick integration with [React Router](https://github.com/reactjs/react-router). Takes precedence over urlSync. See [Custom History](#custom-history).
* @propType {number=700} threshold - Threshold in milliseconds above which new locations will be pushed to the history, instead of replacing the previous one. See [Location Debouncing](#location-debouncing).
Expand Down Expand Up @@ -118,6 +123,7 @@ class InstantSearch extends Component {
appId: props.appId,
apiKey: props.apiKey,
indexName: props.indexName,
configureSearchParameters: props.configureSearchParameters,

initialState,
});
Expand Down Expand Up @@ -208,6 +214,8 @@ InstantSearch.propTypes = {
apiKey: PropTypes.string.isRequired,
indexName: PropTypes.string.isRequired,

configureSearchParameters: PropTypes.func,

history: PropTypes.object,
urlSync: PropTypes.bool,
threshold: PropTypes.number,
Expand Down
13 changes: 13 additions & 0 deletions packages/react-instantsearch/src/core/createConnector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,19 @@ import {omit, has} from 'lodash';

import {shallowEqual, getDisplayName} from './utils';

/**
* @typedef {object} ConnectorDescription
Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure on the context of this addition, can you detail it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started exploring the possibility of writing the widget, and didn't have a proper documentation for the object that could be passed to createConnector. This is its documentation, and I thought maybe we could keep it since it was already done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure, if it currently does not have any output in our documentation website I would say we do not need it for now.

* @property {string} displayName - the displayName used by the wrapper
* @property {function} refine - a function to filter the local state
* @property {function} getSearchParameters - function transforming the local state to a SearchParameters
* @property {function} getMetadata - metadata of the widget
* @property {function} transitionState - hook after the state has changed
* @property {function} getProps - transform the state into props passed to the wrapped component.
* Receives (props, widgetStates, searchState, metadata) and returns the local state.
* @property {object} propTypes - PropTypes forwarded to the wrapped component.
* @property {object} defaultProps - default values for the props
*/

/**
* Connectors are the HOC used to transform React components
* into InstantSearch widgets.
Expand Down
8 changes: 4 additions & 4 deletions packages/react-instantsearch/src/core/createInstantSearchManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import algoliasearchHelper, {SearchParameters} from 'algoliasearch-helper';
import createWidgetsManager from './createWidgetsManager';
import createStore from './createStore';

function identity(x) { return x; }

export default function createInstantSearchManager({
appId,
apiKey,
indexName,
initialState,
configureSearchParameters = identity,
}) {
const client = algoliasearch(appId, apiKey);
const helper = algoliasearchHelper(client);
Expand Down Expand Up @@ -40,10 +43,7 @@ export default function createInstantSearchManager({

function search() {
const baseSP = new SearchParameters({index: indexName});
// @TODO: Provide a way to configure base SearchParameters.
// We previously had a `configureSearchParameters : SP -> SP` option.
// We could also just have a `baseSearchParameters : SP` option.
const searchParameters = getSearchParameters(baseSP);
const searchParameters = configureSearchParameters(getSearchParameters(baseSP));

helper.searchOnce(searchParameters)
.then(({content}) => {
Expand Down