Skip to content

Commit

Permalink
fix(current-refinements): remove alphabetic sorting (#3249)
Browse files Browse the repository at this point in the history
This removes the alphabetic sorting of refinements in CurrentRefinement items. The refinements are sorted based on when the user refines them (default behavior).

Note that the CurrentRefinements items sorting is determined by the order of the [`getRefinements` statements](https://github.com/algolia/instantsearch.js/blob/011d47909894837f2c2da5088bc81803135d201b/src/lib/utils.js#L249). The query being lastly added.

The tests ignore the order of the CurrentRefinement items because there's no specifications for that.
  • Loading branch information
francoischalifour authored Oct 31, 2018
1 parent 2a416f8 commit 9914f87
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,26 @@ describe('connectCurrentRefinements', () => {
createURL: () => '#',
});

expect(rendering.mock.calls[0][0].items).toEqual([
expect.objectContaining({
attribute: 'facet1',
}),
expect.objectContaining({
attribute: 'facet2',
}),
expect.objectContaining({
attribute: 'facet3',
}),
]);
expect(rendering.mock.calls[0][0].items).toEqual(
expect.arrayContaining([
expect.objectContaining({
attribute: 'facet1',
}),
expect.objectContaining({
attribute: 'facet2',
}),
expect.objectContaining({
attribute: 'facet3',
}),
])
);
expect(rendering.mock.calls[0][0].items).toEqual(
expect.not.arrayContaining([
expect.objectContaining({
attribute: 'query',
}),
])
);
});

it('includes only the `includedAttributes`', () => {
Expand Down Expand Up @@ -259,20 +268,22 @@ describe('connectCurrentRefinements', () => {
createURL: () => '#',
});

expect(rendering.mock.calls[0][0].items).toEqual([
expect.objectContaining({
attribute: 'facet1',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet2',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet3',
transformed: true,
}),
]);
expect(rendering.mock.calls[0][0].items).toEqual(
expect.arrayContaining([
expect.objectContaining({
attribute: 'facet1',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet2',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet3',
transformed: true,
}),
])
);

widget.render({
results: new SearchResults(helper.state, [{}]),
Expand All @@ -281,20 +292,22 @@ describe('connectCurrentRefinements', () => {
createURL: () => '#',
});

expect(rendering.mock.calls[0][0].items).toEqual([
expect.objectContaining({
attribute: 'facet1',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet2',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet3',
transformed: true,
}),
]);
expect(rendering.mock.calls[0][0].items).toEqual(
expect.arrayContaining([
expect.objectContaining({
attribute: 'facet1',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet2',
transformed: true,
}),
expect.objectContaining({
attribute: 'facet3',
transformed: true,
}),
])
);
});

it('sort numeric refinements by numeric value', () => {
Expand Down Expand Up @@ -442,7 +455,7 @@ describe('connectCurrentRefinements', () => {

helper
.addFacetRefinement('facet1', 'facetValue')
.addFacetRefinement('facet2', 'facetValue2');
.addFacetRefinement('facet2', 'facetValue');

widget.render({
results: new SearchResults(helper.state, [{}]),
Expand All @@ -452,14 +465,16 @@ describe('connectCurrentRefinements', () => {
});

const secondRenderingOptions = rendering.mock.calls[1][0];
expect(secondRenderingOptions.items).toEqual([
expect.objectContaining({
attribute: 'facet1',
}),
expect.objectContaining({
attribute: 'facet2',
}),
]);
expect(secondRenderingOptions.items).toEqual(
expect.arrayContaining([
expect.objectContaining({
attribute: 'facet1',
}),
expect.objectContaining({
attribute: 'facet2',
}),
])
);
});
});
});
36 changes: 11 additions & 25 deletions src/connectors/current-refinements/connectCurrentRefinements.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,33 +276,19 @@ function normalizeRefinement(refinement) {
};
}

function compareObjects(a, b, attribute) {
if (a[attribute] === b[attribute]) {
return 0;
}

if (a[attribute] < b[attribute]) {
return -1;
}

return 1;
}

function groupItemsByRefinements(items, helper) {
return items.reduce(
(results, currentItem) =>
[
{
attribute: currentItem.attribute,
refinements: items
.filter(result => result.attribute === currentItem.attribute)
.sort((a, b) =>
compareObjects(a, b, a.type === 'numeric' ? 'value' : 'label')
),
refine: refinement => clearRefinement(helper, refinement),
},
...results.filter(result => result.attribute !== currentItem.attribute),
].sort((a, b) => compareObjects(a, b, 'attribute')),
(results, currentItem) => [
...results.filter(result => result.attribute !== currentItem.attribute),
{
attribute: currentItem.attribute,
refinements: items
.filter(result => result.attribute === currentItem.attribute)
// We want to keep the order of refinements except the numeric ones.
.sort((a, b) => (a.type === 'numeric' ? a.value - b.value : 0)),
refine: refinement => clearRefinement(helper, refinement),
},
],
[]
);
}
21 changes: 10 additions & 11 deletions src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,17 +243,7 @@ function getRefinement(state, type, attributeName, name, resultsFacets) {
}

function getRefinements(results, state, clearsQuery) {
const res =
clearsQuery && state.query && state.query.trim()
? [
{
attributeName: 'query',
type: 'query',
name: state.query,
query: state.query,
},
]
: [];
const res = [];

forEach(state.facetsRefinements, (refinements, attributeName) => {
forEach(refinements, name => {
Expand Down Expand Up @@ -317,6 +307,15 @@ function getRefinements(results, state, clearsQuery) {
res.push({ type: 'tag', attributeName: '_tags', name });
});

if (clearsQuery && state.query && state.query.trim()) {
res.push({
attributeName: 'query',
type: 'query',
name: state.query,
query: state.query,
});
}

return res;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,6 @@ exports[`currentRefinements() render() DOM output renders correctly 1`] = `
}
items={
Array [
Object {
"attribute": "_tags",
"refine": [Function],
"refinements": Array [
Object {
"attribute": "_tags",
"label": "tag1",
"type": "tag",
"value": "tag1",
},
Object {
"attribute": "_tags",
"label": "tag2",
"type": "tag",
"value": "tag2",
},
],
},
Object {
"attribute": "disjunctiveFacet",
"refine": [Function],
"refinements": Array [
Object {
"attribute": "disjunctiveFacet",
"count": 3,
"exhaustive": true,
"label": "disjunctiveFacet-val1",
"type": "disjunctive",
"value": "disjunctiveFacet-val1",
},
Object {
"attribute": "disjunctiveFacet",
"count": 4,
"exhaustive": true,
"label": "disjunctiveFacet-val2",
"type": "disjunctive",
"value": "disjunctiveFacet-val2",
},
],
},
Object {
"attribute": "extraFacet",
"refine": [Function],
Expand Down Expand Up @@ -109,6 +69,28 @@ exports[`currentRefinements() render() DOM output renders correctly 1`] = `
},
],
},
Object {
"attribute": "disjunctiveFacet",
"refine": [Function],
"refinements": Array [
Object {
"attribute": "disjunctiveFacet",
"count": 3,
"exhaustive": true,
"label": "disjunctiveFacet-val1",
"type": "disjunctive",
"value": "disjunctiveFacet-val1",
},
Object {
"attribute": "disjunctiveFacet",
"count": 4,
"exhaustive": true,
"label": "disjunctiveFacet-val2",
"type": "disjunctive",
"value": "disjunctiveFacet-val2",
},
],
},
Object {
"attribute": "hierarchicalFacet",
"refine": [Function],
Expand All @@ -123,6 +105,26 @@ exports[`currentRefinements() render() DOM output renders correctly 1`] = `
},
],
},
Object {
"attribute": "numericFacet",
"refine": [Function],
"refinements": Array [
Object {
"attribute": "numericFacet",
"label": "≥ 1",
"operator": ">=",
"type": "numeric",
"value": 1,
},
Object {
"attribute": "numericFacet",
"label": "≤ 2",
"operator": "<=",
"type": "numeric",
"value": 2,
},
],
},
Object {
"attribute": "numericDisjunctiveFacet",
"refine": [Function],
Expand All @@ -144,22 +146,20 @@ exports[`currentRefinements() render() DOM output renders correctly 1`] = `
],
},
Object {
"attribute": "numericFacet",
"attribute": "_tags",
"refine": [Function],
"refinements": Array [
Object {
"attribute": "numericFacet",
"label": "≥ 1",
"operator": ">=",
"type": "numeric",
"value": 1,
"attribute": "_tags",
"label": "tag1",
"type": "tag",
"value": "tag1",
},
Object {
"attribute": "numericFacet",
"label": "≤ 2",
"operator": "<=",
"type": "numeric",
"value": 2,
"attribute": "_tags",
"label": "tag2",
"type": "tag",
"value": "tag2",
},
],
},
Expand Down

0 comments on commit 9914f87

Please sign in to comment.