Skip to content

Commit

Permalink
Merge pull request #148 from algolia/refactor/hits
Browse files Browse the repository at this point in the history
refactor(hits): Remove unused attributes, set sensible defaults
  • Loading branch information
bobylito committed Sep 30, 2015
2 parents 287b540 + 32ec6b0 commit 2f247ad
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 19 deletions.
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,13 @@ search.addWidget(
* Display the list of results (hits) from the current search
* @param {String|DOMElement} options.container CSS Selector or DOMElement to insert the widget
* @param {Object} [options.templates] Templates to use for the widget
* @param {String|Function} [options.templates.empty=''] Template to use when there are no result
* @param {String|Function} [options.templates.hit=''] Template to use for each result
* @param {String|Function} [options.templates.empty=''] Template to use when there are no results.
* Gets passed the `result` from the API call.
* @param {String|Function} [options.templates.hit=''] Template to use for each result.
* Gets passed the `hit` of the result.
* @param {Object} [options.transformData] Method to change the object passed to the templates
* @param {Function} [options.transformData.empty=''] Method used to change the object passed to the empty template
* @param {Function} [options.transformData.hit=''] Method used to change the object passed to the hit template
* @param {boolean} [hideWhenNoResults=true] Hide the container when no results match
* @param {Number} [hitsPerPage=20] The number of hits to display per page
* @return {Object}
*/
Expand Down
10 changes: 5 additions & 5 deletions components/Hits.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ var Template = require('./Template');

class Hits extends React.Component {
renderWithResults() {
var renderedHits = map(this.props.hits, function(hit) {
var renderedHits = map(this.props.hits, (hit) => {
return (
<this.props.Template templateKey="hit" data={hit} key={hit.objectID} />
);
}, this);
});

return <div>{renderedHits}</div>;
}

renderNoResults() {
return (
<div>
<this.props.Template data={this.props.results} templateKey="empty" />
<this.props.Template templateKey="empty" data={this.props.results} />
</div>
);
}
Expand All @@ -31,9 +31,9 @@ class Hits extends React.Component {
}

Hits.propTypes = {
Template: React.PropTypes.func,

This comment has been minimized.

Copy link
@vvo

vvo Sep 30, 2015

Contributor

Just to know, why we reorder keys here? I will try to add the key ordering rule to eslint so that we always have the same order

This comment has been minimized.

Copy link
@pixelastic

pixelastic Sep 30, 2015

Contributor

It's a reflex I have, actually. Ordered keys make diff easier to read (we can easily see when a key is added/deleted), also makes visual scanning of a long list of keys easier and avoid adding the same key several times.

This comment has been minimized.

Copy link
@bobylito

bobylito Sep 30, 2015

Author Contributor

Imho we should not do that, because it creates noise in pull request and does not provide value...

This comment has been minimized.

Copy link
@vvo

vvo Sep 30, 2015

Contributor

there's #154 :D

hits: React.PropTypes.arrayOf(React.PropTypes.object),
results: React.PropTypes.object,
Template: React.PropTypes.func
results: React.PropTypes.object
};

Hits.defaultProps = {
Expand Down
2 changes: 1 addition & 1 deletion example/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ search.addWidget(
empty: require('./templates/no-results.html'),
hit: require('./templates/hit.html')
},
hitsPerPage: 6

This comment has been minimized.

Copy link
@vvo

vvo Sep 30, 2015

Contributor

Ah eh I had setup to 6 only to be able to always see the pagination 💃

This comment has been minimized.

Copy link
@pixelastic

pixelastic Sep 30, 2015

Contributor

Whoops, was for my own testing on the pagination also. I don't mind reverting to 6 :)

hitsPerPage: 10
})
);

Expand Down
25 changes: 15 additions & 10 deletions widgets/hits.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,40 @@ var bindProps = require('../decorators/bindProps');
var Template = require('../components/Template');

var defaultTemplates = {
empty: 'No matching objects, try another search',
hit: 'Object #{{objectID}}, this is the default `hits` template, you should provide one'
empty: 'No results',
hit: function(data) {
return JSON.stringify(data, null, 2);
}
};

/**
* Display the list of results (hits) from the current search
* @param {String|DOMElement} options.container CSS Selector or DOMElement to insert the widget
* @param {Object} [options.templates] Templates to use for the widget
* @param {String|Function} [options.templates.empty=''] Template to use when there are no result
* @param {String|Function} [options.templates.hit=''] Template to use for each result
* @param {String|Function} [options.templates.empty=''] Template to use when there are no results.
* Gets passed the `result` from the API call.
* @param {String|Function} [options.templates.hit=''] Template to use for each result.
* Gets passed the `hit` of the result.
* @param {Object} [options.transformData] Method to change the object passed to the templates
* @param {Function} [options.transformData.empty=''] Method used to change the object passed to the empty template
* @param {Function} [options.transformData.hit=''] Method used to change the object passed to the hit template
* @param {boolean} [hideWhenNoResults=true] Hide the container when no results match
* @param {Number} [hitsPerPage=20] The number of hits to display per page
* @return {Object}
*/
function hits({
container = null,
container,
templates = defaultTemplates,
transformData,
hideWhenNoResults = false,
hitsPerPage = 20
}) {
var Hits = require('../components/Hits');

var containerNode = utils.getContainerNode(container);
var usage = 'Usage: hits({container, [templates.{empty,hit}, transformData.{empty,hit}, hitsPerPage])';

if (container === null) {
throw new Error(usage);
}

return {
getConfiguration: () => ({hitsPerPage}),
Expand All @@ -46,11 +53,9 @@ function hits({

React.render(
<Hits
Template={bindProps(Template, templateProps)}
hits={results.hits}
results={results}
Template={bindProps(Template, templateProps)}
hideWhenNoResults={hideWhenNoResults}
hasResults={results.hits.length > 0}
/>,
containerNode
);
Expand Down

0 comments on commit 2f247ad

Please sign in to comment.