-
Notifications
You must be signed in to change notification settings - Fork 525
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
feat(refinementList): show more count #5593
base: master
Are you sure you want to change the base?
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 825f8be:
|
bf50452
to
ef3b18a
Compare
…olia/instantsearch into feat/refinement-list-show-more-count
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great!
I have a few small nitpicky suggestions, otherwise my main remarks are following.
This could have Vue support quite easily now that you have the connectors set up for InstantSearch.js. Could we put that in here?
We could move some of the testing into our common test suite to have less testing code and better consistency. I can take care of this in a stacked PR.
packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts
Outdated
Show resolved
Hide resolved
packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts
Outdated
Show resolved
Hide resolved
packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts
Outdated
Show resolved
Hide resolved
packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts
Outdated
Show resolved
Hide resolved
packages/react-instantsearch-hooks-web/src/ui/ShowMoreButton.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Dhaya <154633+dhayab@users.noreply.github.com>
Thanks, I realised that I had missed the Vue support he other day so I have started looking at this |
const showMoreTotalCount = Math.min( | ||
showMoreLimit, | ||
facetItems.length | ||
); | ||
showMoreCount = showMoreTotalCount - currentLimit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the number given to the template shouldn't be showMoreTotalCount
, as that's the number of items that will be visible. Not "show x more items", but "10 items / 13 items". You can still easily calculate "total - current" to get the difference. What do you think?
@@ -765,6 +768,7 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/menu/js/#co | |||
sendEvent: expect.any(Function), | |||
canRefine: true, | |||
isShowingMore: false, | |||
showMoreCount: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should probably be a test where showMoreCount isn't 0
Co-authored-by: Dhaya <154633+dhayab@users.noreply.github.com>
…olia/instantsearch into feat/refinement-list-show-more-count
* refactoring: move show more count tests to common suite * map flavor specific props during setup
Hey @mikedavis77, what's the status of this? Do you want to pick it up or should we close the PR? |
Hi @sarahdayan thanks for the nudge on this. Sorry, I realised that I hadn't posted an updated on here, but I need someone who knows Vue to be able to look at updating this for the Vue library (I had a look but have zero Vue experience). Would anyone be available to have a look at this to add the relevant changes to the Vue library along with adding tests? I would love to get this over the line and released. |
Summary
Currently the 'show more' button doesn't provide any information about how many more facet values are available when clicking the 'show more' button.
This behaviour has been asked for several times by customers and raised on this Stackoverflow issue.
Result
Based upon the POC that I did for this Stackoverflow issue using a custom refinementList connector, I have applied this to the standard refinementList, HierarchicalMenu and Menu connectors. This adds an additional param that is passed to the
showMoreText
template function so that it can be used within the templating of these buttons.While this began with the refinementList, this has expanded to also update the hierarchicalMenu and menu widgets as these use the same showMore component.
Examples of how this can be used is: