fix(toggle): pass isRefined to toggleRefinement

Was broken since perf improvements: we did not wanted to bind the function with isRefined in render() to avoid generating new function references each time. But we still need it, so passed all the way down from RefinementListItem
algolia · Feb 20, 2016 · 8ac494e · 8ac494e
1 parent 5ff87f5
commit 8ac494e
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 28 deletions.
diff --git a/src/components/RefinementList/RefinementList.js b/src/components/RefinementList/RefinementList.js
@@ -19,8 +19,8 @@ class RefinementList extends React.Component {
     return nextState !== this.state || !isEqual(this.props.facetValues, nextProps.facetValues);
   }
 
-  refine(value) {
-    this.props.toggleRefinement(value);
+  refine(facetValueToRefine, isRefined) {
+    this.props.toggleRefinement(facetValueToRefine, isRefined);
   }
 
   _generateFacetItem(facetValue) {
@@ -53,8 +53,9 @@ class RefinementList extends React.Component {
 
     return (
       <RefinementListItem
-        facetValue={facetValue[this.props.attributeNameKey]}
+        facetValueToRefine={facetValue[this.props.attributeNameKey]}
         handleClick={this.handleItemClick}
+        isRefined={facetValue.isRefined}
         itemClassName={cssClassItem}
         key={key}
         subItems={subItems}
@@ -80,36 +81,36 @@ class RefinementList extends React.Component {
   //
   // Finally, we always stop propagation of the event to avoid multiple levels RefinementLists to fail: click
   // on child would click on parent also
-  handleItemClick(value, e) {
-    if (isSpecialClick(e)) {
+  handleItemClick({facetValueToRefine, originalEvent, isRefined}) {
+    if (isSpecialClick(originalEvent)) {
       // do not alter the default browser behavior
       // if one special key is down
       return;
     }
 
-    if (e.target.tagName === 'INPUT') {
-      this.refine(value);
+    if (originalEvent.target.tagName === 'INPUT') {
+      this.refine(facetValueToRefine, isRefined);
       return;
     }
 
-    let parent = e.target;
+    let parent = originalEvent.target;
 
-    while (parent !== e.currentTarget) {
+    while (parent !== originalEvent.currentTarget) {
       if (parent.tagName === 'LABEL' && (parent.querySelector('input[type="checkbox"]')
           || parent.querySelector('input[type="radio"]'))) {
         return;
       }
 
       if (parent.tagName === 'A' && parent.href) {
-        e.preventDefault();
+        originalEvent.preventDefault();
       }
 
       parent = parent.parentNode;
     }
 
-    e.stopPropagation();
+    originalEvent.stopPropagation();
 
-    this.refine(value);
+    this.refine(facetValueToRefine, isRefined);
   }
 
   handleClickShowMore() {

diff --git a/src/components/RefinementList/RefinementListItem.js b/src/components/RefinementList/RefinementListItem.js
@@ -12,8 +12,12 @@ class RefinementListItem extends React.Component {
     return !isEqual(this.props, nextProps);
   }
 
-  handleClick(e) {
-    this.props.handleClick(this.props.facetValue, e);
+  handleClick(originalEvent) {
+    this.props.handleClick({
+      facetValueToRefine: this.props.facetValueToRefine,
+      isRefined: this.props.isRefined,
+      originalEvent
+    });
   }
 
   render() {
@@ -34,11 +38,12 @@ class RefinementListItem extends React.Component {
 }
 
 RefinementListItem.propTypes = {
-  facetValue: React.PropTypes.oneOfType([
+  facetValueToRefine: React.PropTypes.oneOfType([
     React.PropTypes.string,
     React.PropTypes.number
   ]).isRequired,
   handleClick: React.PropTypes.func.isRequired,
+  isRefined: React.PropTypes.bool.isRequired,
   itemClassName: React.PropTypes.string,
   subItems: React.PropTypes.object,
   templateData: React.PropTypes.object.isRequired,

diff --git a/src/components/RefinementList/__tests__/RefinementList-test.js b/src/components/RefinementList/__tests__/RefinementList-test.js
@@ -33,17 +33,22 @@ describe('RefinementList', () => {
     parentListProps = {className: 'list'};
     itemProps = [{
       ...commonItemProps,
-      facetValue: 'facet1',
+      itemClassName: 'item active',
+      facetValueToRefine: 'facet1',
+      isRefined: true,
       templateData: {
         ...templateData,
-        name: 'facet1'
+        name: 'facet1',
+        isRefined: true
       }
     }, {
       ...commonItemProps,
-      facetValue: 'facet2',
+      facetValueToRefine: 'facet2',
+      isRefined: false,
       templateData: {
         ...templateData,
-        name: 'facet2'
+        name: 'facet2',
+        isRefined: false
       }
     }];
     renderer = createRenderer();
@@ -63,8 +68,8 @@ describe('RefinementList', () => {
         />
       </div>
     );
-    expect(out.props.children[0][0].key).toEqual('facet1');
-    expect(out.props.children[0][1].key).toEqual('facet2');
+    expect(out.props.children[0][0].key).toEqual('facet1/true');
+    expect(out.props.children[0][1].key).toEqual('facet2/false');
   });
 
   it('should render default list highlighted', () => {
@@ -74,7 +79,6 @@ describe('RefinementList', () => {
       count: 42,
       isRefined: true
     };
-    itemProps[0].itemClassName += ' active';
     expect(out).toEqualJSX(
       <div {...parentListProps}>
         <RefinementListItem
@@ -110,11 +114,13 @@ describe('RefinementList', () => {
         cssClasses: {
           depth: 'depth',
           item: 'item',
-          list: 'list'
+          list: 'list',
+          active: 'active'
         },
         facetValues: [
           {
             name: 'facet1',
+            isRefined: true,
             data: [
               {name: 'subfacet1'},
               {name: 'subfacet2'}
@@ -164,8 +170,8 @@ describe('RefinementList', () => {
         active: 'active'
       },
       facetValues: [
-        {name: 'facet1'},
-        {name: 'facet2'}
+        {name: 'facet1', isRefined: true},
+        {name: 'facet2', isRefined: false}
       ],
       ...extraProps
     };

diff --git a/src/widgets/toggle/__tests__/toggle-test.js b/src/widgets/toggle/__tests__/toggle-test.js
@@ -253,10 +253,10 @@ describe('toggle()', () => {
       let values;
 
       function toggleOn() {
-        widget.toggleRefinement(helper, false);
+        widget.toggleRefinement(helper, 'facetValueToRefine', false);
       }
       function toggleOff() {
-        widget.toggleRefinement(helper, true);
+        widget.toggleRefinement(helper, 'facetValueToRefine', true);
       }
 
       beforeEach(() => {

diff --git a/src/widgets/toggle/toggle.js b/src/widgets/toggle/toggle.js
@@ -113,7 +113,7 @@ function toggle({
         helper.addFacetRefinement(attributeName, userValues.off);
       }
     },
-    toggleRefinement: (helper, isRefined) => {
+    toggleRefinement: (helper, facetValue, isRefined) => {
       let on = userValues.on;
       let off = userValues.off;