Skip to content

Commit

Permalink
fix(sortBy): ensure a return value for getWidgetSearchParameters (#4126)
Browse files Browse the repository at this point in the history
This PR ensures that the `sortBy` widget always return a value for `index` (or at least don't remove the current one). The call to `getWidgetSearchParameters` might happen once before the `init` step. The current implementation relies on the `init` step (`this.initialIndex`) when no value is found in the `uiState`. When the widget is not initialized the value set is `undefined` which means that it removes the current index.

To avoid this problem we fall back on the current index when none of the conditions are met. It's the responsibility of the widget to set a value. It doesn't make sense to return `SearchParameters` without an index.
  • Loading branch information
samouss authored and Haroenv committed Oct 23, 2019
1 parent 952dc70 commit 569d573
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
28 changes: 27 additions & 1 deletion src/connectors/sort-by/__tests__/connectSortBy-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/sort-by/js/
);
});

test('should enforce the default value on empty UiState', () => {
test('should return the initial index on empty UiState with widget initialized', () => {
const [widget, helper, refine] = getInitializedWidget();

refine('other');
Expand All @@ -549,6 +549,32 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/sort-by/js/
})
);
});

test('should return the current index on empty UiState without widget initialized', () => {
const sortBy = connectSortBy(() => {});
const widget = sortBy({
items: [
{ label: 'Sort products by relevance', value: 'relevance' },
{ label: 'Sort products by price', value: 'priceASC' },
{ label: 'Sort products by magic', value: 'other' },
],
});

const actual = widget.getWidgetSearchParameters(
new SearchParameters({
index: 'relevance',
}),
{
uiState: {},
}
);

expect(actual).toEqual(
new SearchParameters({
index: 'relevance',
})
);
});
});
});
});
2 changes: 1 addition & 1 deletion src/connectors/sort-by/connectSortBy.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export default function connectSortBy(renderFn, unmountFn = noop) {
getWidgetSearchParameters(searchParameters, { uiState }) {
return searchParameters.setQueryParameter(
'index',
uiState.sortBy || this.initialIndex
uiState.sortBy || this.initialIndex || searchParameters.index
);
},
};
Expand Down

0 comments on commit 569d573

Please sign in to comment.