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

fix(MultiIndex): handle if namespace isn't in search state #139

Merged
merged 2 commits into from
Jun 26, 2017
Merged

fix(MultiIndex): handle if namespace isn't in search state #139

merged 2 commits into from
Jun 26, 2017

Conversation

mthuret
Copy link
Contributor

@mthuret mthuret commented Jun 22, 2017 •
edited
Loading

Use case: you handle a tab navigation without any router and you're switching between a multiple indices display and several only one indices display.
At some point, before the rendering of only one indices, the namespace with the index name is not here yet, so without this check it crashed.

@algobot
Copy link
Contributor

algobot commented Jun 22, 2017 •
edited
Loading

Deploy preview ready!

Built with commit bb30157

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

@mthuret
Copy link
Contributor Author

mthuret commented Jun 23, 2017

@oliviertassinari I'm not sure to understand the argos error here. If you check our storybook here: https://deploy-preview-139--react-instantsearch.netlify.com/react-instantsearch/storybook/ you can see that the stories are fine.

@oliviertassinari
Copy link
Contributor

@mthuret Right, we have a cropping issue with Happo during the screenshot generation.

@mthuret
Copy link
Contributor Author

mthuret commented Jun 23, 2017 •
edited
Loading

@oliviertassinari also it seems that sometimes it takes a screenshot before the return of results.

@vvo
Copy link
Contributor

vvo commented Jun 26, 2017

When would this happen as a user?

@mthuret
Copy link
Contributor Author

mthuret commented Jun 26, 2017

@vvo what do you mean?

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jun 26, 2017 •
edited
Loading

@vvo I don't think that it's a react-instantsearch issue, sounds more like a Happo one, not measuring the component dimensions at the right time. For instance, it's could be linked to asynchronous image loading.

@vvo
Copy link
Contributor

vvo commented Jun 26, 2017 •
edited
Loading

Thanks for the explanation @mthuret. Sorry @oliviertassinari I was just asking Marie more details on the bug usecase.

vvo
vvo approved these changes Jun 26, 2017
View reviewed changes
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

@mthuret mthuret merged commit 1aab324 into master Jun 26, 2017
@mthuret mthuret deleted the fix/multi-index-get-current-refinementValues branch June 26, 2017 13:04
mthuret pushed a commit that referenced this pull request Jun 26, 2017
v4.0.5
b9b7594 
<a name="4.0.5"></a>
## [4.0.5](v4.0.4...v4.0.5) (2017-06-26)

### Bug Fixes

* **MultiIndex:** ensure getResults return only hits matching index in the context (#136) ([124ffe6](124ffe6))
* **MultiIndex:** handle if namespace isn't in search state (#139) ([1aab324](1aab324))
* **storybook:** process CSS through autoprefixer (#138) ([62cf512](62cf512))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vvo vvo vvo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mthuret @algobot @oliviertassinari @vvo