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

fix(StarRatings): always show the stars below #929

Merged
merged 5 commits into from
Feb 5, 2018

Conversation

samouss
Copy link
Collaborator

@samouss samouss commented Jan 31, 2018

Summary

Fix #674

In the current implementation we can have sparse rows when the results only return one facet value or if some numeric filters (ex: rating > 3) are applied. Now all the rows under the highest value are filled. Since the v5 is in beta I didn't update the tests for avoid to much conflict at the merge. But we should get rid of the react-test-renderer for enzyme. Same for the component we could move to a SFC after the merge.

Before:

screen shot 2018-01-31 at 16 24 48

After:

screen shot 2018-01-31 at 16 24 37

You can play with the widget on Storybook.

@algobot
Copy link
Contributor

algobot commented Jan 31, 2018

Deploy preview for react-instantsearch ready!

Built with commit 7b2c249

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

@Haroenv
Copy link
Contributor

Haroenv commented Feb 1, 2018

@samouss nothing seems to happen in the playground now 🤔

@samouss samouss force-pushed the fix/star-ratings-show-up-only branch 5 times, most recently from 1ff6ef9 to 0fcbcf2 Compare February 2, 2018 14:03
@samouss
Copy link
Collaborator Author

samouss commented Feb 5, 2018

@Haroenv @vvo Any blockers?

@samouss samouss force-pushed the fix/star-ratings-show-up-only branch from 0fcbcf2 to 4064e0c Compare February 5, 2018 10:09
isLastSelectableItem: range.length - 1 === index,
max: limitMax,
translate,
createURL,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is refine not in this object anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Take a look at the signature of buildItem refine is not used. We access the function through the props.

buildItem({
max,
lowerBound,
count,
translate,
createURL,
isLastSelectableItem,
}) {

@samouss samouss force-pushed the fix/star-ratings-show-up-only branch from 4064e0c to 7b2c249 Compare February 5, 2018 11:47
Copy link
Contributor

@vvo vvo left a comment

Choose a reason for hiding this comment

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

LGTM this seems logical now

@samouss samouss merged commit 22bf93a into master Feb 5, 2018
@samouss samouss deleted the fix/star-ratings-show-up-only branch February 5, 2018 13:17
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.

None yet

4 participants