Skip to content

Commit 6635304

Browse files
authored
fix(connectHitsPerPage): default value should not break the API (#3006)
* refactor(test): used jest method for readability * chore(test): reproduce #2732 * fix(connectHitsPerPage): default value should not break the API * fix(connectHitsPerPage): provide a meaningful default value
1 parent 00353ae commit 6635304

File tree

2 files changed

+92
-4
lines changed

2 files changed

+92
-4
lines changed

src/connectors/hits-per-page/__tests__/connectHitsPerPage-test.js

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ describe('connectHitsPerPage', () => {
212212
const firstRenderingOptions = rendering.lastCall.args[0];
213213
expect(firstRenderingOptions.items).toHaveLength(3);
214214
firstRenderingOptions.refine(firstRenderingOptions.items[0].value);
215-
expect(helper.getQueryParameter('hitsPerPage')).toBe(undefined);
215+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
216216

217217
// Reset the hitsPerPage to an actual value
218218
helper.setQueryParameter('hitsPerPage', 7);
@@ -227,7 +227,93 @@ describe('connectHitsPerPage', () => {
227227
const secondRenderingOptions = rendering.lastCall.args[0];
228228
expect(secondRenderingOptions.items).toHaveLength(3);
229229
secondRenderingOptions.refine(secondRenderingOptions.items[0].value);
230-
expect(helper.getQueryParameter('hitsPerPage')).toBe(undefined);
230+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
231+
});
232+
233+
it('the option for unselecting values should work even if stringified', () => {
234+
const rendering = sinon.stub();
235+
const makeWidget = connectHitsPerPage(rendering);
236+
const widget = makeWidget({
237+
items: [
238+
{ value: 3, label: '3 items per page' },
239+
{ value: 10, label: '10 items per page' },
240+
],
241+
});
242+
243+
const helper = jsHelper({}, '', {
244+
hitsPerPage: 7,
245+
});
246+
helper.search = sinon.stub();
247+
248+
widget.init({
249+
helper,
250+
state: helper.state,
251+
createURL: () => '#',
252+
onHistoryChange: () => {},
253+
});
254+
255+
const firstRenderingOptions = rendering.lastCall.args[0];
256+
expect(firstRenderingOptions.items).toHaveLength(3);
257+
firstRenderingOptions.refine(`${firstRenderingOptions.items[0].value}`);
258+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
259+
260+
// Reset the hitsPerPage to an actual value
261+
helper.setQueryParameter('hitsPerPage', 7);
262+
263+
widget.render({
264+
results: new SearchResults(helper.state, [{}]),
265+
state: helper.state,
266+
helper,
267+
createURL: () => '#',
268+
});
269+
270+
const secondRenderingOptions = rendering.lastCall.args[0];
271+
expect(secondRenderingOptions.items).toHaveLength(3);
272+
secondRenderingOptions.refine(`${secondRenderingOptions.items[0].value}`);
273+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
274+
});
275+
276+
it('Should be able to unselect using an empty string', () => {
277+
const rendering = sinon.stub();
278+
const makeWidget = connectHitsPerPage(rendering);
279+
const widget = makeWidget({
280+
items: [
281+
{ value: 3, label: '3 items per page' },
282+
{ value: 10, label: '10 items per page' },
283+
],
284+
});
285+
286+
const helper = jsHelper({}, '', {
287+
hitsPerPage: 7,
288+
});
289+
helper.search = sinon.stub();
290+
291+
widget.init({
292+
helper,
293+
state: helper.state,
294+
createURL: () => '#',
295+
onHistoryChange: () => {},
296+
});
297+
298+
const firstRenderingOptions = rendering.lastCall.args[0];
299+
expect(firstRenderingOptions.items).toHaveLength(3);
300+
firstRenderingOptions.refine('');
301+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
302+
303+
// Reset the hitsPerPage to an actual value
304+
helper.setQueryParameter('hitsPerPage', 7);
305+
306+
widget.render({
307+
results: new SearchResults(helper.state, [{}]),
308+
state: helper.state,
309+
helper,
310+
createURL: () => '#',
311+
});
312+
313+
const secondRenderingOptions = rendering.lastCall.args[0];
314+
expect(secondRenderingOptions.items).toHaveLength(3);
315+
secondRenderingOptions.refine('');
316+
expect(helper.getQueryParameter('hitsPerPage')).not.toBeDefined();
231317
});
232318

233319
describe('routing', () => {

src/connectors/hits-per-page/connectHitsPerPage.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,13 @@ The first one will be picked, you should probably set only one default value`
157157
);
158158
}
159159

160-
items = [{ value: undefined, label: '' }, ...items];
160+
items = [{ value: '', label: '' }, ...items];
161161
}
162162

163163
this.setHitsPerPage = value =>
164-
helper.setQueryParameter('hitsPerPage', value).search();
164+
!value && value !== 0
165+
? helper.setQueryParameter('hitsPerPage', undefined).search()
166+
: helper.setQueryParameter('hitsPerPage', value).search();
165167

166168
renderFn(
167169
{

0 commit comments

Comments
 (0)