Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix range slider pips and value 0 #2350

Merged
merged 4 commits into from
Sep 20, 2017
Merged

Conversation

iam4x
Copy link
Contributor

@iam4x iam4x commented Sep 20, 2017

Summary

  • When setting pips: false it was still showing pips
  • Refine on range boundaries where not correctly applied

Fixes #2343

@iam4x iam4x requested a review from bobylito September 20, 2017 10:08
@algobot
Copy link
Contributor

algobot commented Sep 20, 2017

Deploy preview ready!

Built with commit 0dd17d3

https://deploy-preview-2350--algolia-instantsearch.netlify.com

@bobylito
Copy link
Contributor

I have a question @iam4x , why does the header disappear when there is no pips? Is that a behaviour that we want?

@iam4x
Copy link
Contributor Author

iam4x commented Sep 20, 2017

I just did not put a header on the second example, it's expected behaviour 👍

Copy link
Contributor

@bobylito bobylito left a comment

Choose a reason for hiding this comment

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

I'd add some more tests with different without the bounds.

expect(helper.getNumericRefinement('price', '<=')).toEqual([30]);
expect(helper.search.callCount).toBe(1);

refine([0, undefined]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for the sake of being exhaustive, maybe we could try without the bounds. And also do the third case with [undefined, 20]

@bobylito bobylito dismissed their stale review September 20, 2017 14:13

Updated with latest changes

Copy link
Contributor

@bobylito bobylito left a comment

Choose a reason for hiding this comment

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

Cool fix. That's a tough one 👍

@bobylito bobylito merged commit fa0dc09 into develop Sep 20, 2017
@bobylito bobylito deleted the fix-range-slider-pips-and-value-0 branch September 20, 2017 14:26
bobylito pushed a commit that referenced this pull request Sep 25, 2017
<a name=2.1.5></a>
## [2.1.5](v2.1.4...v2.1.5) (2017-09-25)

### Bug Fixes

* **deps:** update dependency algolia-frontend-components to v^0.0.33 ([#2341](#2341)) ([16994d8](16994d8))
* **price-ranges:** update call to refine ([#2377](#2377)) ([34915d7](34915d7))
* **slider:** Fix range slider pips and value 0 ([#2350](#2350)) ([fa0dc09](fa0dc09)), closes [#2343](#2343)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants