Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(connectRange): use the same behaviour for currentRefinement in getMetadata #923

Merged
merged 1 commit into from
Feb 5, 2018

Conversation

samouss
Copy link
Collaborator

@samouss samouss commented Jan 29, 2018

Summary

Close #920

We provide two different values for the currentRefinement in getProvidedProps & getMetadata. In the first case we "normalize" the value with the range, in the latter we pass the raw values. It can lead to some issues when the user use the currentRefinement from the connectCurrentRefinement since the values aren't aligned.

Edit: At some point we should avoid to "normalize" the currentRefinement with the range from the connector. Passing the raw values to the view and let it decide which value to render is more flexible. It's a bit more work to do in the view but the connector will be more reusable (ex: We can implement a "correct" behaviour for RatingMenu).

@samouss samouss requested review from Haroenv and vvo January 29, 2018 17:13
@algobot
Copy link
Contributor

algobot commented Jan 29, 2018

Deploy preview for react-instantsearch ready!

Built with commit 9a5ad23

https://deploy-preview-923--react-instantsearch.netlify.com

@@ -470,7 +470,7 @@ describe('connectRange', () => {
{
label: '5 <= wot',
attributeName: 'wot',
currentRefinement: { min: 5, max: undefined },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so does undefined not work anymore to refine with only min?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope I didn't update the refine behaviour. I only have updated the currentRefinements value provided by the getMetadata function.

@samouss samouss force-pushed the fix/connect-range-getmetadata branch from 203504e to a476c13 Compare February 2, 2018 14:04
@samouss
Copy link
Collaborator Author

samouss commented Feb 5, 2018

@Haroenv @vvo Any blockers?

@samouss samouss force-pushed the fix/connect-range-getmetadata branch from a476c13 to 9a5ad23 Compare February 5, 2018 10:10
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems correct to me

@samouss samouss merged commit 08917b6 into master Feb 5, 2018
@samouss samouss deleted the fix/connect-range-getmetadata branch February 5, 2018 11:45
samouss added a commit that referenced this pull request Feb 6, 2018
<a name="4.5.0"></a>
# [4.5.0](v4.4.2...v4.5.0) (2018-02-06)

### Bug Fixes

* **connectRange:** use the same behaviour for currentRefinement in getMetadata ([#923](#923)) ([08917b6](08917b6))
* **connectToggle:** use currentRefinement in metadata instead of the label ([#909](#909)) ([89cae2b](89cae2b))
* **StarRatings:** always show the stars below ([#929](#929)) ([22bf93a](22bf93a))

### Features

* **connectStateResults:** expose isSearchStalled ([#933](#933)) ([f45ba27](f45ba27))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants