Skip to content

Commit

Permalink
feat: Add Toggle example
Browse files Browse the repository at this point in the history
  • Loading branch information
pixelastic committed Sep 8, 2015
1 parent 3b2a450 commit d801807
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 14 deletions.
34 changes: 22 additions & 12 deletions components/Toggle.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,36 @@
var React = require('react');

var Template = require('./Template');
var debounce = require('lodash/function/debounce');

class Toggle extends React.Component {
render() {
var toggleFilter = this.props.toggleFilter;
var label = this.props.label;
var isRefined = this.props.isRefined;
var template = this.props.template;
// When a checkbox is wrapped inside a label, click events fire twice, so we
// debounce it to only keep the first one
var toggleFilter = debounce(this.props.toggleFilter, 0, {
leading: true,
trailing: false
});

This comment has been minimized.

Copy link
@bobylito

bobylito Sep 8, 2015

Contributor

Interesting trick! Have you tested on multiple browsers?

This comment has been minimized.

Copy link
@vvo

vvo Sep 8, 2015

Contributor

I must add I am against using timers here even if "it should always work".

Because the "always" is based on JavaScript timers being run and multiple of them being launched. And JavaScript timers are tied to the user environment and current performance/usage.

So I would prefer to solve the "checbkox inside label" case with the code I did in multipleChoiceList: https://github.com/algolia/instantsearch.js/blob/77434f08163f878b95e74ad52895bf3fd27d8bd0/components/MultipleChoiceList.js#L10-L41

I know this is more code (well it's less code in fact if you have a look at lodash debounce): but it's precisely targeting the issue "checkbox in label" specific behavior.

We already had the discussion with @pixelastic but I wanted you to be aware of it too.

This comment has been minimized.

Copy link
@pixelastic

pixelastic Sep 8, 2015

Author Contributor

I understand @vvo point and I'd rather we have only one solution to this issue in our code. I left the debounce so we can test it in real life scenarios. After having tested a few implementation, we'll see pros and cons for both.

This comment has been minimized.

Copy link
@vvo

vvo Sep 8, 2015

Contributor

Yes for this plan

This comment has been minimized.

Copy link
@bobylito

bobylito Sep 8, 2015

Contributor

Well I must say that using debounce is not what I expected. I have to agree with @vvo here, it is better to have a solution that is absolute and not relying on side effects or stateful mechanism. Here we have a behavior that is changed if a call was done before...

Still the hack is interesting :P

This comment has been minimized.

Copy link
@pixelastic

pixelastic Sep 8, 2015

Author Contributor

Yeah, good point. Having a reproducible output when we do automated testing is the way to go. But on the other hand, this bug will only be triggered by real humans using their browser, so we need some kind of integration testing one step higher to test that.

var data = {
label: this.props.label,
isRefined: this.props.isRefined
};

return (
<label>
<input
checked={isRefined}
onChange={toggleFilter}
type="checkbox"
/>
{label}
</label>
<span onClick={toggleFilter}>
<Template data={data} template={template} />
</span>
);
}
}

Toggle.propTypes = {
toggleFilter: React.PropTypes.func,
template: React.PropTypes.oneOfType([
React.PropTypes.string,
React.PropTypes.func
]).isRequired,
toggleFilter: React.PropTypes.func.isRequired,
label: React.PropTypes.string,
isRefined: React.PropTypes.bool
};
Expand Down
9 changes: 9 additions & 0 deletions example/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ search.addWidget(
})
);

search.addWidget(
instantsearch.widgets.toggle({
container: '#free_shipping',
facetName: 'free_shipping',
label: 'Free Shipping',
template: require('./templates/free_shipping.html')
})
);

search.addWidget(
instantsearch.widgets.menu({
container: '#categories',
Expand Down
10 changes: 10 additions & 0 deletions example/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ <h1>Instant search demo <small>using instantsearch.js</small></h1>
<div class="panel-body" id="search-box"></div>
</div>

<div class="panel panel-default">
<div class="panel-heading">Toggle filters</div>
<div class="panel-body">
<ul class="nav nav-stacked">
<li><div id="free_shipping"></div></li>
</ul>
</div>
</div>

<div class="panel panel-default">
<div class="panel-heading">Categories</div>
<div id="categories" class="list-group"></div>
Expand All @@ -27,6 +36,7 @@ <h1>Instant search demo <small>using instantsearch.js</small></h1>
<div class="panel-heading">Brands</div>
<div class="panel-body" id="brands"></div>
</div>

</div>
<div class="col-md-9">
<div id="stats"></div>
Expand Down
9 changes: 9 additions & 0 deletions example/templates/free_shipping.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<div class="checkbox">
<label>
<input type="checkbox" {{#isRefined}}checked{{/isRefined}} />
<span class="badge pull-right">{{label}}</span>
</label>
</div>



1 change: 1 addition & 0 deletions example/templates/hit.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<h3 class="pull-right text-right text-info">${{price}}</h3>
<h4>{{{_highlightResult.name.value}}}</h4>
<p>{{{_highlightResult.description.value}}}</p>
{{#free_shipping}}<span class="badge pull-right">Free Shipping</span>{{/free_shipping}}
</div>
</div>
</div>
7 changes: 5 additions & 2 deletions widgets/toggle.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
var React = require('react');

var utils = require('../lib/widget-utils.js');
var defaultTemplate = '<label>{{label}}<input type="checkbox" {{#isRefined}}checked{{/isRefined}} /></label>';

function toggle({
container = null,
facetName = null,
label = null
label = null,
template = defaultTemplate
}) {
var Toggle = require('../components/Toggle');

Expand All @@ -31,8 +33,9 @@ function toggle({
React.render(
<Toggle
isRefined={isRefined}
toggleFilter={toggleFilter}
label={label}
template={template}
toggleFilter={toggleFilter}
/>,
containerNode
);
Expand Down

0 comments on commit d801807

Please sign in to comment.