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

fix(RefinementList): allow overriding of searchable placeholder #576

Merged
merged 3 commits into from
Nov 13, 2018

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Nov 12, 2018

Also noticed that the test was broken for searchable, so fixed that one too.

New prop: searchable-placeholder on ais-refinement-list

fixes #572

Also noticed that the test was broken for `searchable`, so fixed that one too.

New prop: placeholder on `RefinementList`

fixes #572
@Haroenv Haroenv requested a review from a team November 12, 2018 12:07
@@ -25,6 +25,7 @@ Name | Type | Default | Description | Required
---|---|---|---|---
attribute | string | | The attribute to refine on click | yes
searchable | boolean | `false` | You can also search within the options of this | no
placeholder | string | "Search here …" | If searchable is `true`, this will be the placeholder for that search box | no
Copy link
Contributor

Choose a reason for hiding this comment

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

Placeholder is a bit broad in the context of a RefinementList. Why not searchablePlaceholder ?

Copy link
Member

Choose a reason for hiding this comment

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

We indeed call this searchablePlaceholder in InstantSearch.js 3 (see new options).

@Haroenv Haroenv merged commit d293b83 into v2 Nov 13, 2018
@Haroenv Haroenv deleted the feat/refinementlist-placeholder branch November 13, 2018 09:12
@@ -121,6 +124,11 @@ export default {
type: Boolean,
default: false,
},
searchablePlaceholder: {
default: 'Search here…',
Copy link
Member

Choose a reason for hiding this comment

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

This is not part of the RefinementList spec yet, I'm not sure we want a default placeholder here?

In InstantSearch.js 3 it's "Search..." for now but I would rather go without any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there will always be a placeholder since we use ais-search-box, we should probably spec it

also, go back to rest 👨‍⚕️

Haroenv added a commit to algolia/instantsearch that referenced this pull request Dec 28, 2022
…lia/vue-instantsearch#576)

* fix(RefinementList): allow overriding of searchable placeholder

Also noticed that the test was broken for `searchable`, so fixed that one too.

New prop: placeholder on `RefinementList`

fixes algolia/vue-instantsearch#572

* Update RefinementList.md

* chore: searchablePlaceholder
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.

3 participants