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

HitsPerPage doesn’t work correctly with routeToState() #5397

Closed
joshangell opened this issue May 20, 2019 · 9 comments · Fixed by #5469
Closed

HitsPerPage doesn’t work correctly with routeToState() #5397

joshangell opened this issue May 20, 2019 · 9 comments · Fixed by #5469
Labels
Library: Vue InstantSearch Issues in vue-instantsearch

Comments

@joshangell
Copy link

Bug 🐞

What is the current behavior?

Make a sandbox with the current behavior

Sandbox example: https://codesandbox.io/embed/vueinstantsearchv2starter-8jx9i

What is the expected behavior?

The dropdown should reflect the selected option in the URL when you have selected something other than the default and then reloaded the page.

Does this happen only in specific situations?

What is the proposed solution?

Unsure!

What is the version you are using?

v2.1.0

@francoischalifour
Copy link
Member

Thanks for the bug report.

The issue likely comes from the HitsPerPage component that doesn't pre-select the current refinement, as opposed to the SortBy component.

@joshangell
Copy link
Author

Aha, so if I were to have a crack at a PR for this would using SortBy as a guide be the simplest way forward?

@francoischalifour
Copy link
Member

Yes, it would be!

@drpeck
Copy link

drpeck commented Jul 23, 2019

In addition to not setting the correct value, I've found that the router only updates for changes to the value set at page first load.
E.g. Page loads at /search?hitsPerPage=20, when you select 10 the url changes to /search?hitsPerPage=10, when you select 20 again the url changes to /search.

@drpeck
Copy link

drpeck commented Jul 23, 2019

Setting the initial page loaded value can be achieved by:
<ais-hits-per-page :items="hitsPerPageOptions" />
and

data(){
return {
hitsPerPageOptions: [
        { label: "8 results", value: 8, default: this.$router.currentRoute.query.hitsPerPage != 16 },
        { label: "16 results", value: 16, default: this.$router.currentRoute.query.hitsPerPage == 16 }
      ]
}}

@Haroenv
Copy link
Contributor

Haroenv commented Jul 23, 2019

Thanks for writing this workaround @drpeck

@sarahdayan sarahdayan added the Library: Vue InstantSearch Issues in vue-instantsearch label Dec 23, 2022
@Haroenv Haroenv transferred this issue from algolia/vue-instantsearch Dec 30, 2022
@jonathan-bird
Copy link

jonathan-bird commented Jan 31, 2023

This would still be amazing to have @Haroenv if it's on the cards.

Edit: for anyone looking to do this in 2023, there is an example on how to do it now: https://github.com/algolia/instantsearch/blob/master/examples/vue/e-commerce/src/App.vue#L564

Would still be amazing to have it out of the box though as sort-by is.

Haroenv added a commit that referenced this issue Jan 31, 2023
This makes it responsive to the URL, as expected like other widgets.

fixes #5397

Slight improvement in the HitsPerPage + SortBy tests, as it was relying on Vue state instead of the DOM

I wanted both to be exactly the same, but SortBy doesn't have isRefined on the items apparently.
@Haroenv
Copy link
Contributor

Haroenv commented Jan 31, 2023

This makes sense, I've just opened a PR which fixes it so you won't need that workaround anymore in the future.

@jonathan-bird
Copy link

Too good, thanks!

Haroenv added a commit that referenced this issue Feb 1, 2023
* fix(HitsPerPage): compute selected from isRefined

This makes it responsive to the URL, as expected like other widgets.

fixes #5397

Slight improvement in the HitsPerPage + SortBy tests, as it was relying on Vue state instead of the DOM

I wanted both to be exactly the same, but SortBy doesn't have isRefined on the items apparently.

* deasync
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Library: Vue InstantSearch Issues in vue-instantsearch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants