fix(range): don't go out of bounds with min or max given

[eunjae-lee]

Summary

fixes #4626

This PR fixes the bug where connectRange set the wrong range when min or max was given.

If you look at lines like this, connectRange assumes there is only one value per operator (It's getting the first item from the array). However, in getWidgetSearchParameters, it adds refinements for min and max bound if given. After that, it again adds the bound from uiState.

It's reproduced in controlled mode (by calling setUiState).

Result

It should fixes the reproduction from #4626
The fixed example: https://codesandbox.io/s/confident-pasteur-ocls6?file=/src/app.js

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 01dfce9:

Sandbox	Source
InstantSearch.js	Configuration
sleepy-sid-wsjoo	PR

[eunjae-lee]

Wait, if the one from uiState is out of the bound, then it should keep things in the bound. I will add a new test and update the PR next week.

[Haroenv]

This is indeed correct, but I'm not sure which one of the two should be taken as the source of truth. In theory there's nothing wrong with an out-of-bounds refinement, but then more of the connector is wrong.

[eunjae-lee]

This is indeed correct, but I'm not sure which one of the two should be taken as the source of truth. In theory there's nothing wrong with an out-of-bounds refinement, but then more of the connector is wrong.

Hmm I thought the min, max bound given to the widget should be the source of truth. In the latest change 404d29d, the out-of-bound refinements are capped with the bound.
What do you think?

[Haroenv]

this makes sense I think, although you'll need to change the initial test to make sense

[eunjae-lee]

this makes sense I think, although you'll need to change the initial test to make sense

can you tell me what about the initial test needs to change?

[Haroenv]

change is good, but I don't know what the first test is meant to assert

[eunjae-lee]

change is good, but I don't know what the first test is meant to assert

It might be unclear to you.

      const widget = makeWidget({
        attribute: 'price',
        min: 0,
        max: 500,
      });

      const actual = widget.getWidgetSearchParameters!(helper.state, {
        uiState: {
          range: {
            price: '0:400',
          },
        },
      });

      console.log(actual.numericRefinements.price['<=']);

Before this PR, it used to print [400, 500], instead of [400].